lkml.org 
[lkml]   [2020]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RESEND PATCH v2 1/3] powerpc/powernv: Interface to define support and preference for a SPR
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 <psampat@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 <linuxram@us.ibm.com>
RP

\
 
 \ /
  Last update: 2020-01-13 08:45    [W:0.072 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site