Re: [PATCH v3 2/9] clk: ralink: add clock and reset driver for MTMIPS SoCs

[Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>]

Hi krzysztof,

On Sat, Jun 17, 2023 at 9:11 PM Sergio Paracuellos
<sergio.paracuellos@xxxxxxxxx> wrote:
>
> On Sat, Jun 17, 2023 at 7:20 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >
> > On 17/06/2023 17:37, Sergio Paracuellos wrote:
> > >>> The case of
> > >>> searching for compatible is a mess since as you can see in the
> > >>> bindings there are tons of compatibles to search for, then (this code
> > >>> is common to all ralink platforms).
> > >>
> > >> Compatible is one of the ways using ABI.
> > >
> > > Ok so it is also a broken approach, then.
> >
> > What is exactly broken approach? Fetching by compatibles? Somehow many
> > other platforms do not have problem with that, even for multiple
> > compatibles. Why yours is special?

Ok so fetching by compatible would be a valid approach, then. The
following works for me. Would you also be ok doing it this way?

static const char *clk_cpu(int *idx)
{
    *idx = 1;

    switch (ralink_soc) {
    case RT2880_SOC:
        return "ralink,rt2880-sysc";
    case RT3883_SOC:
        return "ralink,rt3883-sysc";
    case RT305X_SOC_RT3050:
        return "ralink,rt3050-sysc";
    case RT305X_SOC_RT3052:
        return "ralink,rt3052-sysc";
    case RT305X_SOC_RT3350:
        return "ralink,rt3350-sysc";
    case RT305X_SOC_RT3352:
        return "ralink,rt3352-sysc";
    case RT305X_SOC_RT5350:
        return "ralink,rt5350-sysc";
    case MT762X_SOC_MT7620A:
        *idx = 2;
        return "ralink,mt7620a-sysc";
     case MT762X_SOC_MT7620N:
        *idx = 2;
        return "ralink,mt7620-sysc";
     case MT762X_SOC_MT7628AN:
        return "ralink,mt7628-sysc";
     case MT762X_SOC_MT7688:
       return "ralink,mt7688-sysc";
     default:
        *idx = -1;
        return "invalid";
  }
}

void __init plat_time_init(void)
{
    struct of_phandle_args clkspec;
    const char *compatible;
    struct clk *clk;
    int cpu_clk_idx;

    ralink_of_remap();

    compatible = clk_cpu(&cpu_clk_idx);
    if (cpu_clk_idx == -1)
        panic("unable to get CPU clock index");

   of_clk_init(NULL);
   clkspec.np = of_find_compatible_node(NULL, NULL, compatible);
   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);
   mips_hpt_frequency = clk_get_rate(clk) / 2;
   clk_put(clk);
   timer_probe();
}

Thanks,
    Sergio Paracuellos
>
> I guess it is not special but I cannot figure out the way of getting
> this clock using compatibles...
>
> >
> > Anyway, it is not a correct way to get clocks frequency. There is CCF
> > for this, although maybe Ralink does not support it?
>
> This means to use clk_get() if I understand properly but it does not
> work at all for ralink...
>
> >
> > Best regards,
> > Krzysztof
> >
>
> Thanks,
>     Sergio Paracuellos