[PATCH v4 3/4] riscv: cpu: fixes to display proper CPU features

[Bin Meng]

Hi Simon,

On Sun, Jun 21, 2020 at 9:10 PM Sagar Shrikant Kadam
<sagar.kadam at sifive.com> wrote:
>
> The cmd "cpu detail" fetches uninitialized cpu feature information
> and thus displays wrong / inconsitent details as below. FU540-C000 doesn't
> have any microcode, yet the cmd display's it.
> => cpu detail
>   0: cpu at 0      rv64imac
>         ID = 0, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>   1: cpu at 1      rv64imafdc
>         ID = 1, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>   2: cpu at 2      rv64imafdc
>         ID = 2, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>   3: cpu at 3      rv64imafdc
>         ID = 3, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>   4: cpu at 4      rv64imafdc
>         ID = 4, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID
>         Microcode version 0x0
>         Device ID 0x0
>
> The L1 cache or MMU entry seen above is also displayed inconsistently.
> So initialize features to zero before fetching from device tree.
> Additionally the conditional check to read "mmu-type" from device tree
> is not rightly handled due to which the cpu feature doesn't include
> CPU_FEAT_MMU even if it's corresponding entry is present in device tree.
>
> We now see correct features as:
>
> => cpu detail
>   0: cpu at 0      rv64imac
>         ID = 0, freq = 999.100 MHz
>   1: cpu at 1      rv64imafdc
>         ID = 1, freq = 999.100 MHz: MMU
>   2: cpu at 2      rv64imafdc
>         ID = 2, freq = 999.100 MHz: MMU
>   3: cpu at 3      rv64imafdc
>         ID = 3, freq = 999.100 MHz: MMU
>   4: cpu at 4      rv64imafdc
>         ID = 4, freq = 999.100 MHz: MMU
>
> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com>
> Reviewed-by: Pragnesh Patel <pragnesh.patel at sifive.com>
> ---
>  drivers/cpu/riscv_cpu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
> index 76b0489..8c4b5e7 100644
> --- a/drivers/cpu/riscv_cpu.c
> +++ b/drivers/cpu/riscv_cpu.c
> @@ -38,6 +38,8 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
>
>         /* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */
>         info->cpu_freq = 0;
> +       /* Initialise cpu features before updating from device tree */
> +       info->features = 0;

For this one, do you think we should fix the cpu_get_info() in
cpu-uclass driver instead? With fix in the cpu-uclass driver we can
avoid similar issue in any single CPU driver.

>
>         /* First try getting the frequency from the assigned clock */
>         ret = clk_get_by_index(dev, 0, &clk);
> @@ -52,7 +54,7 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info)
>                 dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq);
>
>         mmu = dev_read_string(dev, "mmu-type");
> -       if (!mmu)
> +       if (mmu)
>                 info->features |= BIT(CPU_FEAT_MMU);
>
>         return 0;

Regards,
Bin