lkml.org 
[lkml]   [2020]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC 1/3] powerpc/powernv: Interface to define support and preference for a SPR
From
Date
Hello Ram,

Thank you for your reviewing the patches.

>> +/* 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 8
>> +#define PREFERENCE_MASK 0xff
>> +
>> +#define UNSUPPORTED 0x0
>> +#define SELF_RESTORE_STRICT 0x01
>> +#define SELF_SAVE_STRICT 0x10
>> +
>> +/*
>> + * Bitmask defining the kind of preferences available.
>> + * Note : The higher to lower preference is from LSB to MSB, with a shift of
>> + * 8 bits.
> A minor comment.
>
> Is there a reason why shift is 8? Shift of 4 must be sufficient,
> and a mask of '0xf' should do. And SELF_SAVE_STRICT can be 0x2.
>
>
Yes, you're right! We could do away with using fewer bits here.

>> +/* Caching the lpcr & ptcr support to use later */
>> +static bool is_lpcr_self_save;
>> +static bool is_ptcr_self_save;
> I understand why you need to track the status of PTCR register.
> But its not clear, why LPCR register's save status need to be tracked?
>
Normally it does not but LPCR was previously unsupported in self-restore
and the kernel saved and restored its value in context. Now that we have
support for saving LPCR automatically I believe we leverage it and
make sure the kernel does not do redundant work.

>> +
>> +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,
>> + },
>> + {
>> + .spr = SPRN_HMEER,
>> + .preferred_mode = PREFER_RESTORE_SAVE,
>> + .supported_mode = SELF_RESTORE_STRICT,
>> + },
>> + {
>> + .spr = SPRN_HID0,
>> + .preferred_mode = PREFER_RESTORE_SAVE,
>> + .supported_mode = SELF_RESTORE_STRICT,
>> + },
>> + {
>> + .spr = P9_STOP_SPR_MSR,
>> + .preferred_mode = PREFER_RESTORE_SAVE,
>> + .supported_mode = SELF_RESTORE_STRICT,
>> + },
>> + {
>> + .spr = P9_STOP_SPR_PSSCR,
>> + .preferred_mode = PREFER_SAVE_RESTORE,
>> + .supported_mode = SELF_RESTORE_STRICT,
>> + },
>> + {
>> + .spr = SPRN_HID1,
>> + .preferred_mode = PREFER_RESTORE_SAVE,
>> + .supported_mode = SELF_RESTORE_STRICT,
>> + },
>> + {
>> + .spr = SPRN_HID4,
>> + .preferred_mode = PREFER_RESTORE_SAVE,
>> + .supported_mode = SELF_RESTORE_STRICT,
>> + },
>> + {
>> + .spr = SPRN_HID5,
>> + .preferred_mode = PREFER_RESTORE_SAVE,
>> + .supported_mode = SELF_RESTORE_STRICT,
>> + }
>> +};
> What determines the list of registers tracked in this table?
>
>
> .snip..
>
This list is of the SPRs of all the registers that the kernel is interested in
at wakeup. It has been refactored out as a list from what the kernel used
previously in the kernel.


\
 
 \ /
  Last update: 2020-01-06 10:48    [W:0.074 / U:0.500 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site