[PATCH v7 0/4] update clock handler and proper cpu features

U-Boot cmd "cpu detail" shows inconsistent CPU features. The current "cpu detail" sometimes shows "Microcode" as a feature, which is not the case with FU540-C000 on HiFive Unleashed board.
Patch 1: add cpu node aliases. Patch 2: Init CPU information to avoid inconsistent cpu information. Patch 3: Correctly parse and update mmu-type. Patch 4: Set L1 Cache feature if either i-cache or d-cache is present
I have picked few dependent patches from Sean's series from here [1] and [2].
These have applied on mainline U-Boot commit eae62ae8de18 ("Merge tag 'efi-2020-07-rc6' of https://gitlab.denx.de/u-boot/custodians/u-boot-efi")
Patch history: ============================================= v7: -Included the nits suggestion.
V6: -Rebase series with master -Split patch 2 from v5 as suggested i.e init cpu feature in cpu-class.c as one patch and handle mmu-type check into another. -Updated Reviewed-by tags
V5: -Addressed review comments on v4. 1. Removed patch 1 which was for debug message. 2. Updated commit logs with proper information on number of cpu's 3. Additionally used d-cache to set the L1 feature bit.
V4: 1. Rebased the series to mainline commit. 2. Updated dependency list as few patches are now merged. 3. Added U-Boot log of the flow i.e fsbl + fw_payload.bin (Opensbi+U-Boot)
V3: 1. Included the cosmetic change as suggested s/L1 feature/L1 cache feature/ 2. Added Reviewed-By tags
V2: 1. Incorporate review comments from Bin and Sean Anderson. and dropped 2nd patch as similar work was already done in [1] and [2] 2 Add cpu node aliases to display cpu node's in sequence. 3. Add fix to show mmu as available cpu feature. 4. Check and append L1 cache feature.
V1: Base version Thanks to Vincent Chen vincent.chen@sifive.com for testing the V1 version of this series.
[1] https://patchwork.ozlabs.org/patch/1316066 [2] https://patchwork.ozlabs.org/patch/1316067
All these together is available here: https://github.com/sagsifive/u-boot/commits/dev/sagark/clk-v7
U-Boot log: =========================================================== U-Boot SPL 2020.07-rc5-00063-g213d48a (Jun 28 2020 - 07:19:43 -0700) Trying to boot from MMC1
U-Boot 2020.07-rc5-00063-g213d48a (Jun 28 2020 - 07:19:43 -0700)
CPU: rv64imafdc Model: SiFive HiFive Unleashed A00 DRAM: 8 GiB MMC: spi@10050000:mmc@0: 0 In: serial@10010000 Out: serial@10010000 Err: serial@10010000 Net: eth0: ethernet@10090000 Hit any key to stop autoboot: 0 => => cpu list 1: cpu@1 rv64imafdc 2: cpu@2 rv64imafdc 3: cpu@3 rv64imafdc 4: cpu@4 rv64imafdc => cpu detail 1: cpu@1 rv64imafdc ID = 1, freq = 999.100 MHz: L1 cache, MMU 2: cpu@2 rv64imafdc ID = 2, freq = 999.100 MHz: L1 cache, MMU 3: cpu@3 rv64imafdc ID = 3, freq = 999.100 MHz: L1 cache, MMU 4: cpu@4 rv64imafdc ID = 4, freq = 999.100 MHz: L1 cache, MMU =>
Sagar Shrikant Kadam (4): riscv: dts: hifive-unleashed-a00: add cpu aliases uclass: cpu: fix to display proper CPU features riscv: cpu: correctly handle the setting of CPU_FEAT_MMU bit riscv: cpu: check and append L1 cache to cpu features
arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ drivers/cpu/cpu-uclass.c | 3 +++ drivers/cpu/riscv_cpu.c | 17 +++++++++++++---- 3 files changed, 20 insertions(+), 4 deletions(-)

Add cpu aliases to U-Boot specific dtsi for hifive-unleashed. Without aliases we see that the CPU device sequence numbers are set randomly and the cpu list/detail command will show it as follows: => cpu list 1: cpu@1 rv64imafdc 2: cpu@2 rv64imafdc 3: cpu@3 rv64imafdc 0: cpu@4 rv64imafdc
Seems like CPU probing with dm-model also relies on aliases as observed in case spi. The fu540-c000-u-boot.dtsi has cpu nodes and so adding corresponding aliases we can ensure that cpu devices are assigned proper sequence as follows:
=> cpu list 1: cpu@1 rv64imafdc 2: cpu@2 rv64imafdc 3: cpu@3 rv64imafdc 4: cpu@4 rv64imafdc
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com Reviewed-by: Pragnesh Patel pragnesh.patel@sifive.com Reviewed-by: Bin Meng bin.meng@windriver.com --- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index 3038064..e037150 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -8,6 +8,10 @@
/ { aliases { + cpu1 = &cpu1; + cpu2 = &cpu2; + cpu3 = &cpu3; + cpu4 = &cpu4; spi0 = &qspi0; spi2 = &qspi2; };

The cmd "cpu detail" fetches uninitialized cpu feature information and thus displays wrong / inconsitent details as below. For eg: FU540-C000 doesn't have any microcode, yet the cmd display's it.
=> cpu detail 1: cpu@1 rv64imafdc ID = 1, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID Microcode version 0x0 Device ID 0x0 2: cpu@2 rv64imafdc ID = 2, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID Microcode version 0x0 Device ID 0x0 3: cpu@3 rv64imafdc ID = 3, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID Microcode version 0x0 Device ID 0x0 4: cpu@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 cpu information to zero into cpu-uclass itself so that similar issues can be avoided for other CPU drivers.
We now see correct features as: => cpu detail 1: cpu@1 rv64imafdc ID = 1, freq = 999.100 MHz 2: cpu@2 rv64imafdc ID = 2, freq = 999.100 MHz 3: cpu@3 rv64imafdc ID = 3, freq = 999.100 MHz 4: cpu@4 rv64imafdc ID = 4, freq = 999.100 MHz
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com Reviewed-by: Pragnesh Patel pragnesh.patel@sifive.com Reviewed-by: Bin Meng bin.meng@windriver.com --- drivers/cpu/cpu-uclass.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/cpu/cpu-uclass.c b/drivers/cpu/cpu-uclass.c index 7418c26..cbb4419 100644 --- a/drivers/cpu/cpu-uclass.c +++ b/drivers/cpu/cpu-uclass.c @@ -86,6 +86,9 @@ int cpu_get_info(struct udevice *dev, struct cpu_info *info) if (!ops->get_info) return -ENOSYS;
+ /* Init cpu_info to 0 */ + memset(info, 0, sizeof(struct cpu_info)); + return ops->get_info(dev, info); }

The conditional check to read "mmu-type" from the 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 the device tree.
The initialization of cpu features is now taken care in cpu-uclass driver, so no need to zero out cpu_freq in riscv_cpu driver and can be removed.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com Reviewed-by: Pragnesh Patel pragnesh.patel@sifive.com Reviewed-by: Bin Meng bin.meng@windriver.com --- drivers/cpu/riscv_cpu.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c index 76b0489..112690f 100644 --- a/drivers/cpu/riscv_cpu.c +++ b/drivers/cpu/riscv_cpu.c @@ -36,9 +36,6 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info) struct clk clk; const char *mmu;
- /* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */ - info->cpu_freq = 0; - /* First try getting the frequency from the assigned clock */ ret = clk_get_by_index(dev, 0, &clk); if (!ret) { @@ -52,7 +49,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;

All cpu cores within FU540-C000 having split I/D caches. Set the L1 cache feature bit using the i-cache-size or d-cache-size as one of the property from device tree indicating that L1 cache is present on the cpu core.
=> cpu detail 1: cpu@1 rv64imafdc ID = 1, freq = 999.100 MHz: L1 cache, MMU 2: cpu@2 rv64imafdc ID = 2, freq = 999.100 MHz: L1 cache, MMU 3: cpu@3 rv64imafdc ID = 3, freq = 999.100 MHz: L1 cache, MMU 4: cpu@4 rv64imafdc ID = 4, freq = 999.100 MHz: L1 cache, MMU
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com Reviewed-by: Pragnesh Patel Pragnesh.patel@sifive.com Reviewed-by: Bin Meng bin.meng@windriver.com --- drivers/cpu/riscv_cpu.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c index 112690f..100fe55 100644 --- a/drivers/cpu/riscv_cpu.c +++ b/drivers/cpu/riscv_cpu.c @@ -35,6 +35,8 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info) int ret; struct clk clk; const char *mmu; + u32 i_cache_size; + u32 d_cache_size;
/* First try getting the frequency from the assigned clock */ ret = clk_get_by_index(dev, 0, &clk); @@ -52,6 +54,16 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info) if (mmu) info->features |= BIT(CPU_FEAT_MMU);
+ /* check if I cache is present */ + ret = dev_read_u32(dev, "i-cache-size", &i_cache_size); + if (ret) + /* if not found check if d-cache is present */ + ret = dev_read_u32(dev, "d-cache-size", &d_cache_size); + + /* if either I or D cache is present set L1 cache feature */ + if (!ret) + info->features |= BIT(CPU_FEAT_L1_CACHE); + return 0; }
participants (1)
-
Sagar Shrikant Kadam