| Message ID | 20230617052435.359177-9-sergio.paracuellos@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mips: ralink: add complete clock and reset driver for mtmips SoCs | expand |
On 17/06/2023 07:24, Sergio Paracuellos wrote: > At very early stage on boot, there is a need to set 'mips_hpt_frequency'. > This timer frequency is a half of the CPU frequency. To get clocks properly > set we need to call to 'of_clk_init()' and properly get cpu clock frequency > afterwards. Depending on the SoC, CPU clock index in the clock provider is > different being two for MT7620 SoC and one for the rest. Hence, adapt code > to be aligned with new clock driver. > void __init plat_time_init(void) > { > + struct of_phandle_args clkspec; > struct clk *clk; > + int cpu_clk_idx; > > ralink_of_remap(); > > - ralink_clk_init(); > - clk = clk_get_sys("cpu", NULL); > + cpu_clk_idx = clk_cpu_index(); > + if (cpu_clk_idx == -1) > + panic("unable to get CPU clock index"); > + > + of_clk_init(NULL); > + clkspec.np = of_find_node_by_name(NULL, "sysc"); > + clkspec.args_count = 1; > + clkspec.args[0] = cpu_clk_idx; > + clk = of_clk_get_from_provider(&clkspec); This is very obfuscated way of getting clock. Why can't you get it from "clocks" property of "cpu", like every other recent platform? Anyway, NAK for of_find_node_by_name(), because you now create ABI for node name. It's broken approach. Best regards, Krzysztof
On Sat, Jun 17, 2023 at 4:48 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 17/06/2023 07:24, Sergio Paracuellos wrote: > > At very early stage on boot, there is a need to set 'mips_hpt_frequency'. > > This timer frequency is a half of the CPU frequency. To get clocks properly > > set we need to call to 'of_clk_init()' and properly get cpu clock frequency > > afterwards. Depending on the SoC, CPU clock index in the clock provider is > > different being two for MT7620 SoC and one for the rest. Hence, adapt code > > to be aligned with new clock driver. > > > > void __init plat_time_init(void) > > { > > + struct of_phandle_args clkspec; > > struct clk *clk; > > + int cpu_clk_idx; > > > > ralink_of_remap(); > > > > - ralink_clk_init(); > > - clk = clk_get_sys("cpu", NULL); > > + cpu_clk_idx = clk_cpu_index(); > > + if (cpu_clk_idx == -1) > > + panic("unable to get CPU clock index"); > > + > > + of_clk_init(NULL); > > + clkspec.np = of_find_node_by_name(NULL, "sysc"); > > + clkspec.args_count = 1; > > + clkspec.args[0] = cpu_clk_idx; > > + clk = of_clk_get_from_provider(&clkspec); > > This is very obfuscated way of getting clock. Why can't you get it from > "clocks" property of "cpu", like every other recent platform? I did not find any other approach that works for me. So I ended up in this one. Can you point me out in a sample of code doing the same so I can check if it works for me then? > > Anyway, NAK for of_find_node_by_name(), because you now create ABI for > node name. It's broken approach. I will change whatever is needed to provide a valid approach. Please, point me out in the right direction. > > Best regards, > Krzysztof > Thanks in advance for your time. Best regards, Sergio Paracuellos
On 17/06/2023 17:35, Sergio Paracuellos wrote: > On Sat, Jun 17, 2023 at 4:48 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 17/06/2023 07:24, Sergio Paracuellos wrote: >>> At very early stage on boot, there is a need to set 'mips_hpt_frequency'. >>> This timer frequency is a half of the CPU frequency. To get clocks properly >>> set we need to call to 'of_clk_init()' and properly get cpu clock frequency >>> afterwards. Depending on the SoC, CPU clock index in the clock provider is >>> different being two for MT7620 SoC and one for the rest. Hence, adapt code >>> to be aligned with new clock driver. >> >> >>> void __init plat_time_init(void) >>> { >>> + struct of_phandle_args clkspec; >>> struct clk *clk; >>> + int cpu_clk_idx; >>> >>> ralink_of_remap(); >>> >>> - ralink_clk_init(); >>> - clk = clk_get_sys("cpu", NULL); >>> + cpu_clk_idx = clk_cpu_index(); >>> + if (cpu_clk_idx == -1) >>> + panic("unable to get CPU clock index"); >>> + >>> + of_clk_init(NULL); >>> + clkspec.np = of_find_node_by_name(NULL, "sysc"); >>> + clkspec.args_count = 1; >>> + clkspec.args[0] = cpu_clk_idx; >>> + clk = of_clk_get_from_provider(&clkspec); >> >> This is very obfuscated way of getting clock. Why can't you get it from >> "clocks" property of "cpu", like every other recent platform? > > I did not find any other approach that works for me. So I ended up in this one. > Can you point me out in a sample of code doing the same so I can check > if it works for me then? You mean bindings? git grep cpus.yaml Driver? git grep clk_get_rate clk_get eventually of_clk_get It all depends on the context. Anyway, it is very easy to find existing solutions not using of_find_node_by_name for your platform: git grep mips_hpt_frequency First result. Best regards, Krzysztof
On Sat, Jun 17, 2023 at 7:27 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 17/06/2023 17:35, Sergio Paracuellos wrote: > > On Sat, Jun 17, 2023 at 4:48 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 17/06/2023 07:24, Sergio Paracuellos wrote: > >>> At very early stage on boot, there is a need to set 'mips_hpt_frequency'. > >>> This timer frequency is a half of the CPU frequency. To get clocks properly > >>> set we need to call to 'of_clk_init()' and properly get cpu clock frequency > >>> afterwards. Depending on the SoC, CPU clock index in the clock provider is > >>> different being two for MT7620 SoC and one for the rest. Hence, adapt code > >>> to be aligned with new clock driver. > >> > >> > >>> void __init plat_time_init(void) > >>> { > >>> + struct of_phandle_args clkspec; > >>> struct clk *clk; > >>> + int cpu_clk_idx; > >>> > >>> ralink_of_remap(); > >>> > >>> - ralink_clk_init(); > >>> - clk = clk_get_sys("cpu", NULL); > >>> + cpu_clk_idx = clk_cpu_index(); > >>> + if (cpu_clk_idx == -1) > >>> + panic("unable to get CPU clock index"); > >>> + > >>> + of_clk_init(NULL); > >>> + clkspec.np = of_find_node_by_name(NULL, "sysc"); > >>> + clkspec.args_count = 1; > >>> + clkspec.args[0] = cpu_clk_idx; > >>> + clk = of_clk_get_from_provider(&clkspec); > >> > >> This is very obfuscated way of getting clock. Why can't you get it from > >> "clocks" property of "cpu", like every other recent platform? > > > > I did not find any other approach that works for me. So I ended up in this one. > > Can you point me out in a sample of code doing the same so I can check > > if it works for me then? > > You mean bindings? > git grep cpus.yaml > > Driver? > git grep clk_get_rate > clk_get > eventually of_clk_get > > It all depends on the context. > > Anyway, it is very easy to find existing solutions not using > of_find_node_by_name for your platform: > > git grep mips_hpt_frequency > > First result. I have tested before all of these and got into panic because clock cannot get retrieved: For example first result is to make use of clk_get so change the code into: void __init plat_time_init(void) { struct clk *clk; of_clk_init(NULL); clk = clk_get(NULL, "cpu"); if (IS_ERR(clk)) panic("unable to get CPU clock, err=%ld", PTR_ERR(clk)); pr_info("CPU Clock: %ldMHz\n", clk_get_rate(clk) / 1000000); mips_hpt_frequency = clk_get_rate(clk) / 2; clk_put(clk); timer_probe(); } And I panic because I cannot get the cpu clock... > > > Best regards, > Krzysztof > Thanks, Sergio Paracuellos
diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c index 5b02bb7e0829..3d29e956f785 100644 --- a/arch/mips/ralink/clk.c +++ b/arch/mips/ralink/clk.c @@ -11,29 +11,41 @@ #include <linux/clkdev.h> #include <linux/clk.h> #include <linux/clk-provider.h> +#include <asm/mach-ralink/ralink_regs.h> #include <asm/time.h> #include "common.h" -void ralink_clk_add(const char *dev, unsigned long rate) +static int clk_cpu_index(void) { - struct clk *clk = clk_register_fixed_rate(NULL, dev, NULL, 0, rate); + if (ralink_soc == RALINK_UNKNOWN) + return -1; - if (!clk) - panic("failed to add clock"); + if (ralink_soc == MT762X_SOC_MT7620A || + ralink_soc == MT762X_SOC_MT7620N) + return 2; - clkdev_create(clk, NULL, "%s", dev); + return 1; } void __init plat_time_init(void) { + struct of_phandle_args clkspec; struct clk *clk; + int cpu_clk_idx; ralink_of_remap(); - ralink_clk_init(); - clk = clk_get_sys("cpu", NULL); + cpu_clk_idx = clk_cpu_index(); + if (cpu_clk_idx == -1) + panic("unable to get CPU clock index"); + + of_clk_init(NULL); + clkspec.np = of_find_node_by_name(NULL, "sysc"); + clkspec.args_count = 1; + clkspec.args[0] = cpu_clk_idx; + clk = of_clk_get_from_provider(&clkspec); if (IS_ERR(clk)) panic("unable to get CPU clock, err=%ld", PTR_ERR(clk)); pr_info("CPU Clock: %ldMHz\n", clk_get_rate(clk) / 1000000);
At very early stage on boot, there is a need to set 'mips_hpt_frequency'. This timer frequency is a half of the CPU frequency. To get clocks properly set we need to call to 'of_clk_init()' and properly get cpu clock frequency afterwards. Depending on the SoC, CPU clock index in the clock provider is different being two for MT7620 SoC and one for the rest. Hence, adapt code to be aligned with new clock driver. Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> --- arch/mips/ralink/clk.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)