On 18.01.2022 11:17, Roger Pau Monné wrote:
> On Mon, Sep 06, 2021 at 03:00:46PM +0200, Jan Beulich wrote:
>> --- a/xen/arch/x86/cpu/mwait-idle.c
>> +++ b/xen/arch/x86/cpu/mwait-idle.c
>> @@ -742,6 +742,32 @@ static const struct cpuidle_state dnv_cs
>> {}
>> };
>>
>> +/*
>> + * Note, depending on HW and FW revision, SnowRidge SoC may or may not
>> support
>> + * C6, and this is indicated in the CPUID mwait leaf.
>> + */
>> +static const struct cpuidle_state snr_cstates[] = {
>> + {
>> + .name = "C1",
>
> We usually use names like "C1-SNR" or similar in other cpuidle_state
> structs. Is using plain "C1" intentional here?
I don't know. We're simply following the Linux side change. If we're
unhappy with their naming (it indeed looks inconsistent), then we
ought to make a separate patch on top (and maybe submit that also to
Linux).
>> @@ -954,6 +980,11 @@ static const struct idle_cpu idle_cpu_dn
>> .disable_promotion_to_c1e = 1,
>> };
>>
>> +static const struct idle_cpu idle_cpu_snr = {
>> + .state_table = snr_cstates,
>> + .disable_promotion_to_c1e = true,
>
> This likely wants to be 1 because the type is bool_t.
bool_t is just an alias of bool, so "true" ought to be fine. We may
want to follow Linux in switching to bool altogether - iirc we didn't
have bool yet at the time the driver (or the first commit needing it)
was ported. I guess I'll make a patch ...
Jan
