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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMhs-H-Aw8gdj=ZUep6ZX_9JH0TK9JgDx=sdduNkOROkzEN96g@mail.gmail.com>
Date:   Sun, 18 Jun 2023 07:04:55 +0200
From:   Sergio Paracuellos <sergio.paracuellos@...il.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Shiji Yang <yangshiji66@...look.com>, arinc.unal@...nc9.com,
        devicetree@...r.kernel.org, john@...ozen.org,
        krzysztof.kozlowski+dt@...aro.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mips@...r.kernel.org,
        matthias.bgg@...il.com, mturquette@...libre.com,
        p.zabel@...gutronix.de, robh+dt@...nel.org, sboyd@...nel.org,
        tsbogend@...ha.franken.de
Subject: Re: [PATCH v3 2/9] clk: ralink: add clock and reset driver for MTMIPS SoCs

Hi krzysztof,

On Sat, Jun 17, 2023 at 9:11 PM Sergio Paracuellos
<sergio.paracuellos@...il.com> wrote:
>
> On Sat, Jun 17, 2023 at 7:20 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@...aro.org> 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