On 5/2/20 6:06 AM, Pragnesh Patel wrote:
The cpu clock is probably already enabled if we are executing code (though we could be executing from a different core). This patch prevents the cpu clock or its parents from being disabled.
Signed-off-by: Sean Anderson seanga2@gmail.com
If you make substantial changes can you please make a note of it in the commit? I did not sign off on *this* code.
Signed-off-by: Pragnesh Patel pragnesh.patel@sifive.com Reviewed-by: Bin Meng bmeng.cn@gmail.com
drivers/cpu/riscv_cpu.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c index 28ad0aa30f..8ebe0341fd 100644 --- a/drivers/cpu/riscv_cpu.c +++ b/drivers/cpu/riscv_cpu.c @@ -9,6 +9,7 @@ #include <errno.h> #include <dm/device-internal.h> #include <dm/lists.h> +#include <clk.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -100,6 +101,37 @@ static int riscv_cpu_bind(struct udevice *dev) return 0; }
+static int riscv_cpu_probe(struct udevice *dev) +{
- int ret = 0;
- u32 clock = 0;
- struct clk clk;
- /* Get a clock if it exists */
- ret = clk_get_by_index(dev, 0, &clk);
- if (ret)
return 0;- ret = dev_read_u32(dev, "clock-frequency", &clock);
Ok, so as I understand it, your goal for your changes this patch is to have a way to set the cpu frequency on startup. However, I think the cpu-frequency property is not the correct way to go about this when we have a clock from the device tree. In this case, one should use the assigned-clock* properties to have the frequency set automatically when clk_get_by_index is called. There is no need to add this functionality here.
With the previous patch in the series you pulled this from [1], one could easily do something like
cpus { assigned-clocks = <&foo FOO_CPU>; assigned-clock-frequency = <100000000>; cpu@0 { /* ... */ clocks = <&foo FOO_CPU>; /* ... */ }; };
which would use existing code to assign the frequency.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20200423023320.1380090-18-s...
- if (ret) {
debug("clock-frequency not found in dt %d\n", ret);
This should not be an error. You also need to check for ENOSYS and ENOTSUPP like below.
return ret;- } else {
ret = clk_set_rate(&clk, clock);if (ret < 0) {debug("Could not set CPU clock\n");
Neither should this be.
return ret;}- }
- ret = clk_enable(&clk);
- clk_free(&clk);
- if (ret == -ENOSYS || ret == -ENOTSUPP)
All clk_ calls can return ENOSYS and ENOTSUPP. These are returned when the underlying clock does not support the operation. However, they are not appropriate errors to return from this function.
return 0;- else
return ret;+}
static const struct cpu_ops riscv_cpu_ops = { .get_desc = riscv_cpu_get_desc, .get_info = riscv_cpu_get_info, @@ -116,6 +148,7 @@ U_BOOT_DRIVER(riscv_cpu) = { .id = UCLASS_CPU, .of_match = riscv_cpu_ids, .bind = riscv_cpu_bind,
- .probe = riscv_cpu_probe, .ops = &riscv_cpu_ops, .flags = DM_FLAG_PRE_RELOC,
};
--Sean