[PATCH v2 0/4] update clock handler and proper cpu features
U-Boot cmd "cpu detail" shows inconsistent CPU features and is missing clk_request and free handlers. 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 clk request handler to check if valid clock id is requested. Patch 2: add cpu node aliases Patch 3: Correctly parse and update mmu-type.
RISC-V core's on FU540-C000 SoC have separate instruction and data (split) L1 cache. Patch 4:Use i-cache-size dt property as one of identifier to indicate a split cache is available.
I have picked few dependent patches from Sean's and Pragnesh's latest series from here [1]...[5].
These have applied on mainline U-Boot commit 8c48bb21bd6a ("Prepare v2020.07-rc3")
Patch history: ============================================= V2: 1. Incorporate review comments from Bin and Sean Anderson. and dropped 2nd patch as similar work was already done in [1] & [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/1295345 [2] https://patchwork.ozlabs.org/patch/1295346 [3] https://patchwork.ozlabs.org/patch/1297146 [4] https://patchwork.ozlabs.org/patch/1297147 [5] https://patchwork.ozlabs.org/patch/1297149
All these together is available here: https://github.com/sagsifive/u-boot/commits/dev/sagark/clk-v2
Sagar Shrikant Kadam (4): fu540: prci: add request and free clock handlers riscv: dts: hifive-unleashed-a00: add cpu aliases riscv: cpu: fixes to display proper CPU features riscv: cpu: check and append L1 cache to cpu features
arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 5 +++++ drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++ drivers/cpu/riscv_cpu.c | 10 +++++++++- 3 files changed, 35 insertions(+), 1 deletion(-)
Add clk_request handler to check if a valid clock is requested, Here clk_free handler is added for debug purpose which will display details of clock passed to clk_free.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com --- drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c index 67e21b6..bf50ea2 100644 --- a/drivers/clk/sifive/fu540-prci.c +++ b/drivers/clk/sifive/fu540-prci.c @@ -581,6 +581,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk *clk, ulong rate) return rate; }
+static int sifive_fu540_prci_clk_request(struct clk *clk) +{ + debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev, + clk->id); + + if (clk->id >= ARRAY_SIZE(__prci_init_clocks)) + return -EINVAL; + + return 0; +} + +static int sifive_fu540_prci_clk_free(struct clk *clk) +{ + debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev, + clk->id); + + return 0; +} + static int sifive_fu540_prci_probe(struct udevice *dev) { int i, err; @@ -612,6 +631,8 @@ static int sifive_fu540_prci_probe(struct udevice *dev) static struct clk_ops sifive_fu540_prci_ops = { .set_rate = sifive_fu540_prci_set_rate, .get_rate = sifive_fu540_prci_get_rate, + .request = sifive_fu540_prci_clk_request, + .rfree = sifive_fu540_prci_clk_free, };
static const struct udevice_id sifive_fu540_prci_ids[] = {
-----Original Message----- From: Sagar Kadam sagar.kadam@sifive.com Sent: 26 May 2020 22:39 To: u-boot@lists.denx.de; rick@andestech.com; lukma@denx.de Cc: jagan@amarulasolutions.com; bmeng.cn@gmail.com; Pragnesh Patel pragnesh.patel@sifive.com; seanga2@gmail.com; Sagar Kadam sagar.kadam@sifive.com Subject: [PATCH v2 1/4] fu540: prci: add request and free clock handlers
Add clk_request handler to check if a valid clock is requested, Here clk_free handler is added for debug purpose which will display details of clock passed to clk_free.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com
drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
Reviewed-by: Pragnesh Patel pragnesh.patel@sifive.com
diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c index 67e21b6..bf50ea2 100644 --- a/drivers/clk/sifive/fu540-prci.c +++ b/drivers/clk/sifive/fu540-prci.c @@ -581,6 +581,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk *clk, ulong rate) return rate; }
+static int sifive_fu540_prci_clk_request(struct clk *clk) {
- debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
clk->id);- if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
return -EINVAL;- return 0;
+}
+static int sifive_fu540_prci_clk_free(struct clk *clk) {
- debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
clk->id);- return 0;
+}
static int sifive_fu540_prci_probe(struct udevice *dev) { int i, err; @@ -612,6 +631,8 @@ static int sifive_fu540_prci_probe(struct udevice *dev) static struct clk_ops sifive_fu540_prci_ops = { .set_rate = sifive_fu540_prci_set_rate, .get_rate = sifive_fu540_prci_get_rate,
- .request = sifive_fu540_prci_clk_request,
- .rfree = sifive_fu540_prci_clk_free,
};
static const struct udevice_id sifive_fu540_prci_ids[] = {
2.7.4
Hi Pragnesh,
-----Original Message----- From: Pragnesh Patel pragnesh.patel@sifive.com Sent: Wednesday, May 27, 2020 7:41 PM To: Sagar Kadam sagar.kadam@sifive.com; u-boot@lists.denx.de; rick@andestech.com; lukma@denx.de Cc: jagan@amarulasolutions.com; bmeng.cn@gmail.com; seanga2@gmail.com Subject: RE: [PATCH v2 1/4] fu540: prci: add request and free clock handlers
-----Original Message----- From: Sagar Kadam sagar.kadam@sifive.com Sent: 26 May 2020 22:39 To: u-boot@lists.denx.de; rick@andestech.com; lukma@denx.de Cc: jagan@amarulasolutions.com; bmeng.cn@gmail.com; Pragnesh Patel pragnesh.patel@sifive.com; seanga2@gmail.com; Sagar Kadam sagar.kadam@sifive.com Subject: [PATCH v2 1/4] fu540: prci: add request and free clock handlers
Add clk_request handler to check if a valid clock is requested, Here clk_free handler is added for debug purpose which will display details of clock passed to clk_free.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com
drivers/clk/sifive/fu540-prci.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
Reviewed-by: Pragnesh Patel pragnesh.patel@sifive.com
Thanks for the review.
BR, Sagar
diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c index 67e21b6..bf50ea2 100644 --- a/drivers/clk/sifive/fu540-prci.c +++ b/drivers/clk/sifive/fu540-prci.c @@ -581,6 +581,25 @@ static ulong sifive_fu540_prci_set_rate(struct clk *clk, ulong rate) return rate; }
+static int sifive_fu540_prci_clk_request(struct clk *clk) {
- debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
clk->id);- if (clk->id >= ARRAY_SIZE(__prci_init_clocks))
return -EINVAL;- return 0;
+}
+static int sifive_fu540_prci_clk_free(struct clk *clk) {
- debug("%s(clk=%p) (dev=%p, id=%lu)\n", __func__, clk, clk->dev,
clk->id);- return 0;
+}
static int sifive_fu540_prci_probe(struct udevice *dev) { int i, err; @@ -612,6 +631,8 @@ static int sifive_fu540_prci_probe(struct udevice *dev) static struct clk_ops sifive_fu540_prci_ops = { .set_rate = sifive_fu540_prci_set_rate, .get_rate = sifive_fu540_prci_get_rate,
- .request = sifive_fu540_prci_clk_request,
- .rfree = sifive_fu540_prci_clk_free,
};
static const struct udevice_id sifive_fu540_prci_ids[] = {
2.7.4
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@0 rv64imac 0: cpu@1 rv64imafdc 2: cpu@2 rv64imafdc 3: cpu@3 rv64imafdc 4: 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 cpu0/1/2/3/4 nodes and so adding corresponding aliases we can ensure that cpu devices are assigned proper sequence as follows:
=> cpu list 0: cpu@0 rv64imac 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 --- arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi index 9787332..9894260 100644 --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi @@ -7,6 +7,11 @@
/ { aliases { + cpu0 = &cpu0; + cpu1 = &cpu1; + cpu2 = &cpu2; + cpu3 = &cpu3; + cpu4 = &cpu4; spi0 = &qspi0; spi2 = &qspi2; };
-----Original Message----- From: Sagar Kadam sagar.kadam@sifive.com Sent: 26 May 2020 22:39 To: u-boot@lists.denx.de; rick@andestech.com; lukma@denx.de Cc: jagan@amarulasolutions.com; bmeng.cn@gmail.com; Pragnesh Patel pragnesh.patel@sifive.com; seanga2@gmail.com; Sagar Kadam sagar.kadam@sifive.com Subject: [PATCH v2 2/4] riscv: dts: hifive-unleashed-a00: add cpu aliases
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@0 rv64imac 0: cpu@1 rv64imafdc 2: cpu@2 rv64imafdc 3: cpu@3 rv64imafdc 4: 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 cpu0/1/2/3/4 nodes and so adding corresponding aliases we can ensure that cpu devices are assigned proper sequence as follows:
=> cpu list 0: cpu@0 rv64imac 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
arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 5 +++++ 1 file changed, 5 insertions(+)
Reviewed-by: Pragnesh Patel pragnesh.patel@sifive.com
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@0 rv64imac ID = 0, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID Microcode version 0x0 Device ID 0x0 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 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@0 rv64imac ID = 0, freq = 999.100 MHz 1: cpu@1 rv64imafdc ID = 1, freq = 999.100 MHz: MMU 2: cpu@2 rv64imafdc ID = 2, freq = 999.100 MHz: MMU 3: cpu@3 rv64imafdc ID = 3, freq = 999.100 MHz: MMU 4: cpu@4 rv64imafdc ID = 4, freq = 999.100 MHz: MMU
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@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;
/* 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;
-----Original Message----- From: Sagar Kadam sagar.kadam@sifive.com Sent: 26 May 2020 22:39 To: u-boot@lists.denx.de; rick@andestech.com; lukma@denx.de Cc: jagan@amarulasolutions.com; bmeng.cn@gmail.com; Pragnesh Patel pragnesh.patel@sifive.com; seanga2@gmail.com; Sagar Kadam sagar.kadam@sifive.com Subject: [PATCH v2 3/4] riscv: cpu: fixes to display proper CPU features
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@0 rv64imac ID = 0, freq = 999.100 MHz: L1 cache, MMU, Microcode, Device ID Microcode version 0x0 Device ID 0x0 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 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@0 rv64imac ID = 0, freq = 999.100 MHz 1: cpu@1 rv64imafdc ID = 1, freq = 999.100 MHz: MMU 2: cpu@2 rv64imafdc ID = 2, freq = 999.100 MHz: MMU 3: cpu@3 rv64imafdc ID = 3, freq = 999.100 MHz: MMU 4: cpu@4 rv64imafdc ID = 4, freq = 999.100 MHz: MMU
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com
drivers/cpu/riscv_cpu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Pragnesh Patel pragnesh.patel@sifive.com
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;
/* 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;
-- 2.7.4
All cpu cores within FU540-C000 having split I/D caches. Set the L1 feature bit using the i-cache-size as one of the property from device tree indicating that L1 cache is present on the cpu core.
=> cpu detail 0: cpu@0 rv64imac ID = 0, freq = 999.100 MHz: L1 cache 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 --- drivers/cpu/riscv_cpu.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c index 8c4b5e7..ce722cb 100644 --- a/drivers/cpu/riscv_cpu.c +++ b/drivers/cpu/riscv_cpu.c @@ -35,6 +35,7 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info) int ret; struct clk clk; const char *mmu; + u32 split_cache_size;
/* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */ info->cpu_freq = 0; @@ -57,6 +58,11 @@ static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info) if (mmu) info->features |= BIT(CPU_FEAT_MMU);
+ /* check if I/D cache is present */ + ret = dev_read_u32(dev, "i-cache-size", &split_cache_size); + if (!ret) + info->features |= BIT(CPU_FEAT_L1_CACHE); + return 0; }
Hi Sagar,
-----Original Message----- From: Sagar Kadam sagar.kadam@sifive.com Sent: 26 May 2020 22:39 To: u-boot@lists.denx.de; rick@andestech.com; lukma@denx.de Cc: jagan@amarulasolutions.com; bmeng.cn@gmail.com; Pragnesh Patel pragnesh.patel@sifive.com; seanga2@gmail.com; Sagar Kadam sagar.kadam@sifive.com Subject: [PATCH v2 4/4] riscv: cpu: check and append L1 cache to cpu features
All cpu cores within FU540-C000 having split I/D caches. Set the L1 feature bit using the i-cache-size as one of the property from
s/L1 feature/L1 cache feature
device tree indicating that L1 cache is present on the cpu core.
=> cpu detail 0: cpu@0 rv64imac ID = 0, freq = 999.100 MHz: L1 cache 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
Hi Pragnesh,
-----Original Message----- From: Pragnesh Patel pragnesh.patel@sifive.com Sent: Wednesday, May 27, 2020 8:10 PM To: Sagar Kadam sagar.kadam@sifive.com; u-boot@lists.denx.de; rick@andestech.com; lukma@denx.de Cc: jagan@amarulasolutions.com; bmeng.cn@gmail.com; seanga2@gmail.com Subject: RE: [PATCH v2 4/4] riscv: cpu: check and append L1 cache to cpu features
Hi Sagar,
-----Original Message----- From: Sagar Kadam sagar.kadam@sifive.com Sent: 26 May 2020 22:39 To: u-boot@lists.denx.de; rick@andestech.com; lukma@denx.de Cc: jagan@amarulasolutions.com; bmeng.cn@gmail.com; Pragnesh Patel pragnesh.patel@sifive.com; seanga2@gmail.com; Sagar Kadam sagar.kadam@sifive.com Subject: [PATCH v2 4/4] riscv: cpu: check and append L1 cache to cpu features
All cpu cores within FU540-C000 having split I/D caches. Set the L1 feature bit using the i-cache-size as one of the property from
s/L1 feature/L1 cache feature
Ok. Will update
device tree indicating that L1 cache is present on the cpu core.
=> cpu detail 0: cpu@0 rv64imac ID = 0, freq = 999.100 MHz: L1 cache 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
Thanks for the review's
BR, Sagar Kadam
participants (3)
-
Pragnesh Patel -
Sagar Kadam -
Sagar Shrikant Kadam