[PATCH v6 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: ============================================= 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-v6
U-Boot log: =========================================================== U-Boot SPL 2020.07-rc5-00063-g515541f (Jun 26 2020 - 04:44:08 -0700) Trying to boot from MMC1
U-Boot 2020.07-rc5-00063-g515541f (Jun 26 2020 - 04:44:08 -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 --- 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..9f4a8fd 100644 --- a/drivers/cpu/cpu-uclass.c +++ b/drivers/cpu/cpu-uclass.c @@ -83,6 +83,9 @@ int cpu_get_info(struct udevice *dev, struct cpu_info *info) { struct cpu_ops *ops = cpu_get_ops(dev);
+ /* Init cpu_info to 0 */ + memset(info, 0, sizeof(struct cpu_info)); + if (!ops->get_info) return -ENOSYS;
Hi Sagar,
On Fri, Jun 26, 2020 at 8:40 PM Sagar Shrikant Kadam sagar.kadam@sifive.com wrote:
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
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..9f4a8fd 100644 --- a/drivers/cpu/cpu-uclass.c +++ b/drivers/cpu/cpu-uclass.c @@ -83,6 +83,9 @@ int cpu_get_info(struct udevice *dev, struct cpu_info *info) { struct cpu_ops *ops = cpu_get_ops(dev);
/* Init cpu_info to 0 */memset(info, 0, sizeof(struct cpu_info));
nits: this should be put after the if() test logic below
if (!ops->get_info) return -ENOSYS;--
Other than that, Reviewed-by: Bin Meng bin.meng@windriver.com
Regards, Bin
Hi Bin,
-----Original Message----- From: Bin Meng bmeng.cn@gmail.com Sent: Friday, June 26, 2020 6:50 PM To: Sagar Kadam sagar.kadam@sifive.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Rick Chen rick@andestech.com; Bin Meng bin.meng@windriver.com; Jagan Teki jagan@amarulasolutions.com; Pragnesh Patel pragnesh.patel@sifive.com; Anup Patel anup.patel@wdc.com; Simon Glass sjg@chromium.org; Ye Li ye.li@nxp.com; Peng Fan peng.fan@nxp.com; Sean Anderson seanga2@gmail.com Subject: Re: [PATCH v6 2/4] uclass: cpu: fix to display proper CPU features
[External Email] Do not click links or attachments unless you recognize the sender and know the content is safe
Hi Sagar,
On Fri, Jun 26, 2020 at 8:40 PM Sagar Shrikant Kadam sagar.kadam@sifive.com wrote:
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
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..9f4a8fd 100644 --- a/drivers/cpu/cpu-uclass.c +++ b/drivers/cpu/cpu-uclass.c @@ -83,6 +83,9 @@ int cpu_get_info(struct udevice *dev, struct cpu_info *info) { struct cpu_ops *ops = cpu_get_ops(dev);
/* Init cpu_info to 0 */memset(info, 0, sizeof(struct cpu_info));nits: this should be put after the if() test logic below
Ok. I will submit it
if (!ops->get_info) return -ENOSYS;--
Other than that, Reviewed-by: Bin Meng bin.meng@windriver.com
Thanks here.
BR, Sagar
Regards, Bin
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 --- 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;
On Fri, Jun 26, 2020 at 8:41 PM Sagar Shrikant Kadam sagar.kadam@sifive.com wrote:
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
drivers/cpu/riscv_cpu.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Bin Meng bin.meng@windriver.com
Hi,
-----Original Message----- From: Bin Meng bmeng.cn@gmail.com Sent: Friday, June 26, 2020 6:50 PM To: Sagar Kadam sagar.kadam@sifive.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Rick Chen rick@andestech.com; Bin Meng bin.meng@windriver.com; Jagan Teki jagan@amarulasolutions.com; Pragnesh Patel pragnesh.patel@sifive.com; Anup Patel anup.patel@wdc.com; Simon Glass sjg@chromium.org; Ye Li ye.li@nxp.com; Peng Fan peng.fan@nxp.com; Sean Anderson seanga2@gmail.com Subject: Re: [PATCH v6 3/4] riscv: cpu: correctly handle the setting of CPU_FEAT_MMU bit
[External Email] Do not click links or attachments unless you recognize the sender and know the content is safe
On Fri, Jun 26, 2020 at 8:41 PM Sagar Shrikant Kadam sagar.kadam@sifive.com wrote:
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
drivers/cpu/riscv_cpu.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Bin Meng bin.meng@windriver.com
Thanks Bin.
BR, Sagar
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 (3)
-
Bin Meng -
Sagar Kadam -
Sagar Shrikant Kadam