Re: [PATCH] MIPS: Ingenic: Fix bugs when detecting X1000's parameters.

[Zhou Yanjie <zhouyanjie@zoho.com>]

Hi Paul,
On 2019年07月31日 02:02, Paul Cercueil wrote:
> Hi Zhou,
>
>
>
> Le mar. 30 juil. 2019 à 10:55, Zhou Yanjie <zhouyanjie@zoho.com> a 
> écrit :
>> 1.fix bugs when detecting L2 cache sets value.
>> 2.fix bugs when detecting L2 cache ways value.
>> 3.fix bugs when calculate bogoMips and loops_per_jiffy.
>>
>> Signed-off-by: Zhou Yanjie <zhouyanjie@zoho.com>
>> ---
>>  arch/mips/kernel/cpu-probe.c |  7 ++++++-
>>  arch/mips/mm/sc-mips.c       | 18 +++++++++++++++---
>>  2 files changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>> index eb527a1..a914435 100644
>> --- a/arch/mips/kernel/cpu-probe.c
>> +++ b/arch/mips/kernel/cpu-probe.c
>> @@ -1960,11 +1960,16 @@ static inline void cpu_probe_ingenic(struct 
>> cpuinfo_mips *c, unsigned int cpu)
>>      c->options &= ~MIPS_CPU_COUNTER;
>>      BUG_ON(!__builtin_constant_p(cpu_has_counter) || cpu_has_counter);
>>      switch (c->processor_id & PRID_IMP_MASK) {
>> -    case PRID_IMP_XBURST:
>> +    case PRID_IMP_XBURST: {
>> +        unsigned int config7;
>>          c->cputype = CPU_XBURST;
>>          c->writecombine = _CACHE_UNCACHED_ACCELERATED;
>>          __cpu_name[cpu] = "Ingenic XBurst";
>> +        config7 = read_c0_config7();
>> +        config7 |= (1 << 4);
>> +        write_c0_config7(config7);
>
> If you add __BUILD_SET_C0(config7) in arch/mips/include/asm/mipsregs.h
> (search for this macro) then you can call directly 
> set_c0_config7(BIT(4)).
>
> It's preferred to use the BIT(x) macro instead of the (1 << x) construct.
Thanks for your suggestion, I have used set_c0_config7(BIT(4)) instead
of the (1 << x) construct in v2.
> Finally, what does that bit do? I can't find it any documentation about
> it. Please add a comment describing what it does.
According to Ingenic's explanation, in the XBurst architecture this bit is
used to control a low-power mode, which is intended to allow the CPU
to further reduce power consumption, but due to some bugs, it is easy
to cause errors in the calculation of bogomips and loops_per_jiffy, So
Ingenic's suggestion is to set this bit to turn it off.
>
>>          break;
>> +    }
>>      default:
>>          panic("Unknown Ingenic Processor ID!");
>>          break;
>> diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
>> index 9385ddb..ed953d4 100644
>> --- a/arch/mips/mm/sc-mips.c
>> +++ b/arch/mips/mm/sc-mips.c
>> @@ -215,6 +215,14 @@ static inline int __init mips_sc_probe(void)
>>      else
>>          return 0;
>>
>> +    /*
>> +     * According to config2 it would be 512-sets, but that is 
>> contradicted
>> +     * by all documentation.
>> +     */
>> +    if (current_cpu_type() == CPU_XBURST &&
>> +                mips_machtype == MACH_INGENIC_X1000)
>> +        c->scache.sets = 256;
>> +
>>      tmp = (config2 >> 0) & 0x0f;
>>      if (tmp <= 7)
>>          c->scache.ways = tmp + 1;
>> @@ -225,9 +233,13 @@ static inline int __init mips_sc_probe(void)
>>       * According to config2 it would be 5-ways, but that is 
>> contradicted
>>       * by all documentation.
>>       */
>> -    if (current_cpu_type() == CPU_XBURST &&
>> -                mips_machtype == MACH_INGENIC_JZ4770)
>> -        c->scache.ways = 4;
>> +    if (current_cpu_type() == CPU_XBURST) {
>> +        switch (mips_machtype) {
>> +        case MACH_INGENIC_JZ4770:
>> +        case MACH_INGENIC_X1000:
>> +            c->scache.ways = 4;
>> +        }
>> +    }
>>
>>      c->scache.waysize = c->scache.sets * c->scache.linesz;
>>      c->scache.waybit = __ffs(c->scache.waysize);
>> -- 
>> 2.7.4
>>
>>
>
>