Re: [PATCH v7 02/21] target/loongarch: Add core definition

[Philippe Mathieu-Daudé]

On 10/18/21 18:06, WANG Xuerui wrote:
> Hi Song,
> 
> On 10/18/21 20:47, Song Gao wrote:
>> This patch add target state header, target definitions
>> and initialization routines.
> "adds"; fix in other patches too.
>>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/loongarch/cpu-param.h |  19 +++
>>   target/loongarch/cpu.c       | 285
>> +++++++++++++++++++++++++++++++++++++++++++
>>   target/loongarch/cpu.h       | 151 +++++++++++++++++++++++
>>   target/loongarch/internals.h |  23 ++++
>>   4 files changed, 478 insertions(+)
>>   create mode 100644 target/loongarch/cpu-param.h
>>   create mode 100644 target/loongarch/cpu.c
>>   create mode 100644 target/loongarch/cpu.h
>>   create mode 100644 target/loongarch/internals.h


>> +static void set_loongarch_cpucfg(CPULoongArchState *env)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < 49; i++) {
>> +        env->cpucfg[i] = 0x0;
>> +    }
>> +    env->cpucfg[0] = 0x14c010;
>> +    env->cpucfg[1] = 0x3f2f2fe;
>> +    env->cpucfg[2] = 0x60c3cf;
>> +    env->cpucfg[3] = 0xcff;
>> +    env->cpucfg[4] = 0x5f5e100;
>> +    env->cpucfg[5] = 0x10001;
>> +    env->cpucfg[16] = 0x2c3d;
>> +    env->cpucfg[17] = 0x6080003;
>> +    env->cpucfg[18] = 0x6080003;
>> +    env->cpucfg[19] = 0x60800f;
>> +    env->cpucfg[20] = 0x60f000f;
> I know these values are taken from a real 3A5000 chip, since I have such
> a machine and I've done the experiment, but others likely wouldn't
> notice so quickly. Maybe put some comment on top of this function to
> illustrate this?

Simpler: ...

>> +}
>> +
>> +/* LoongArch CPU definitions */
> Doc-string for such a function should be verb phrase; but in this case
> would it be better to just drop the comment? The code is pretty concise
> and self-documenting after all.
>> +static void loongarch_3a5000_initfn(Object *obj)
>> +{
>> +    LoongArchCPU *cpu = LOONGARCH_CPU(obj);
>> +    CPULoongArchState *env = &cpu->env;
>> +
>> +    set_loongarch_cpucfg(env);

... directly inline here.

>> +}