| Message ID | add5b9bd-6f88-6320-427f-7387985af9a4@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | riscv: Try to get cpu frequency from device tree | expand |
Hi Sean, On Fri, 2020-01-17 at 14:51 -0500, Sean Anderson wrote: > Instead of always using the "clock-frequency" property to determine cpu > frequency, try using a clock in "clocks" if it exists. > > Signed-off-by Sean Anderson <seanga2@gmail.com> > --- > This patch depends on <https://patchwork.ozlabs.org/patch/1223933/>;. > > drivers/cpu/riscv_cpu.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c > index 1e32bb5678..280c9de376 100644 > --- a/drivers/cpu/riscv_cpu.c > +++ b/drivers/cpu/riscv_cpu.c > @@ -3,6 +3,7 @@ > * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com> > */ > > +#include <clk.h> > #include <common.h> > #include <cpu.h> > #include <dm.h> > @@ -27,11 +28,24 @@ static int riscv_cpu_get_desc(struct udevice *dev, char *buf, int size) > > static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info) > { > + int err; > + struct clk clk; > const char *mmu; > > /* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */ > info->cpu_freq = 0; > - dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq); > + > + /* First try getting the frequency from the assigned clock */ > + err = clk_get_by_index(dev, 0, &clk); Usually, ret is used as a variable name here. I think it would actually make the code a bit nicer to read here, because the clock rate is not read from variable err. But that's just nit-picking. The patch looks good otherwise! Reviewed-by: Lukas Auer <lukas@auer.io> > + if (!err) { > + err = clk_get_rate(&clk); > + if (!IS_ERR_VALUE(err)) > + info->cpu_freq = err; > + clk_free(&clk); > + } > + > + if (!info->cpu_freq) > + dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq); > > mmu = dev_read_string(dev, "mmu-type"); > if (!mmu)
On 1/26/20 11:34 AM, Lukas Auer wrote: > Hi Sean, > Usually, ret is used as a variable name here. I think it would actually > make the code a bit nicer to read here, because the clock rate is not > read from variable err. Hm, I chose err instead of ret since that variable is never the return value of the function. I can change that for v2 if you'd like. > But that's just nit-picking. The patch looks good otherwise! > > Reviewed-by: Lukas Auer <lukas@auer.io>
On Sun, 2020-01-26 at 13:20 -0500, Sean Anderson wrote: > On 1/26/20 11:34 AM, Lukas Auer wrote: > > Hi Sean, > > Usually, ret is used as a variable name here. I think it would actually > > make the code a bit nicer to read here, because the clock rate is not > > read from variable err. > > Hm, I chose err instead of ret since that variable is never the return > value of the function. I can change that for v2 if you'd like. > Makes sense. I think it's fine to keep it as is. > > But that's just nit-picking. The patch looks good otherwise! > > > > Reviewed-by: Lukas Auer <lukas@auer.io> > >
Hi Sean > From: U-Boot [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Lukas Auer > Sent: Monday, January 27, 2020 6:26 AM > To: u-boot@lists.denx.de; seanga2@gmail.com > Subject: Re: [PATCH] riscv: Try to get cpu frequency from device tree > > On Sun, 2020-01-26 at 13:20 -0500, Sean Anderson wrote: > > On 1/26/20 11:34 AM, Lukas Auer wrote: > > > Hi Sean, > > > Usually, ret is used as a variable name here. I think it would > > > actually make the code a bit nicer to read here, because the clock > > > rate is not read from variable err. > > > > Hm, I chose err instead of ret since that variable is never the return > > value of the function. I can change that for v2 if you'd like. > > > > Makes sense. I think it's fine to keep it as is. But this patch seem depend on [PATCH v2 06/11] riscv: Fix incorrect cpu frequency on RV64. Can you combine those two patches as one patch-set and also modify err as ret BTW. Thanks Rick > > > > But that's just nit-picking. The patch looks good otherwise! > > > > > > Reviewed-by: Lukas Auer <lukas@auer.io>
On 2/4/20 5:36 AM, Bin Meng wrote: > On Mon, Feb 3, 2020 at 1:40 AM Sean Anderson <seanga2@gmail.com> wrote: > I believe both 2 patches in this series are needed by "riscv: Add > Sipeed Maix support" series? > If yes, I think you can just put 2 patches into the same series, to > give people a good context next time. Hm, I had split them off due to the following comment On 1/31/20 12:46 AM, Rick Chen wrote: > But this patch seem depend on [PATCH v2 06/11] riscv: Fix incorrect > cpu frequency on RV64. > Can you combine those two patches as one patch-set and also modify err > as ret BTW. I guess the intention was to combine them *in* the patch series. --Sean
Hi Sean, On Tue, Feb 4, 2020 at 10:06 PM Sean Anderson <seanga2@gmail.com> wrote: > > On 2/4/20 5:36 AM, Bin Meng wrote: > > On Mon, Feb 3, 2020 at 1:40 AM Sean Anderson <seanga2@gmail.com> wrote: > > I believe both 2 patches in this series are needed by "riscv: Add > > Sipeed Maix support" series? > > If yes, I think you can just put 2 patches into the same series, to > > give people a good context next time. > Hm, I had split them off due to the following comment > > On 1/31/20 12:46 AM, Rick Chen wrote: > > But this patch seem depend on [PATCH v2 06/11] riscv: Fix incorrect > > cpu frequency on RV64. > > Can you combine those two patches as one patch-set and also modify err > > as ret BTW. > > I guess the intention was to combine them *in* the patch series. > I meant you can include these 2 patches in the "riscv: Add Sipeed Maix support" series. Regards, Bin
On 2/4/20 9:27 AM, Bin Meng wrote: > Hi Sean, > > On Tue, Feb 4, 2020 at 10:06 PM Sean Anderson <seanga2@gmail.com> wrote: >> >> On 2/4/20 5:36 AM, Bin Meng wrote: >>> On Mon, Feb 3, 2020 at 1:40 AM Sean Anderson <seanga2@gmail.com> wrote: >>> I believe both 2 patches in this series are needed by "riscv: Add >>> Sipeed Maix support" series? >>> If yes, I think you can just put 2 patches into the same series, to >>> give people a good context next time. >> Hm, I had split them off due to the following comment >> >> On 1/31/20 12:46 AM, Rick Chen wrote: >>> But this patch seem depend on [PATCH v2 06/11] riscv: Fix incorrect >>> cpu frequency on RV64. >>> Can you combine those two patches as one patch-set and also modify err >>> as ret BTW. >> >> I guess the intention was to combine them *in* the patch series. >> > > I meant you can include these 2 patches in the "riscv: Add Sipeed > Maix support" series. Yes, I can do that in the next revision, sorry for the confusion.
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c index 1e32bb5678..280c9de376 100644 --- a/drivers/cpu/riscv_cpu.c +++ b/drivers/cpu/riscv_cpu.c @@ -3,6 +3,7 @@ * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com> */ +#include <clk.h> #include <common.h> #include <cpu.h> #include <dm.h> @@ -27,11 +28,24 @@ static int riscv_cpu_get_desc(struct udevice *dev, char *buf, int size) static int riscv_cpu_get_info(struct udevice *dev, struct cpu_info *info) { + int err; + struct clk clk; const char *mmu; /* Zero out the frequency, in case sizeof(ulong) != sizeof(u32) */ info->cpu_freq = 0; - dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq); + + /* First try getting the frequency from the assigned clock */ + err = clk_get_by_index(dev, 0, &clk); + if (!err) { + err = clk_get_rate(&clk); + if (!IS_ERR_VALUE(err)) + info->cpu_freq = err; + clk_free(&clk); + } + + if (!info->cpu_freq) + dev_read_u32(dev, "clock-frequency", (u32 *)&info->cpu_freq); mmu = dev_read_string(dev, "mmu-type"); if (!mmu)