linux-kernel - Re: [PATCH v13 4/6] clk: meson: a1: add Amlogic A1 PLL clock controller driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAFBinCA3uZXzr3RgnWnKV5Qr-CPaZQX5joDg319i_cgzhLJy2g@mail.gmail.com>
Date:   Sun, 23 Apr 2023 23:12:27 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Dmitry Rokosov <ddrokosov@...rdevices.ru>
Cc:     neil.armstrong@...aro.org, jbrunet@...libre.com,
        mturquette@...libre.com, sboyd@...nel.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, khilman@...libre.com,
        jian.hu@...ogic.com, kernel@...rdevices.ru, rockosov@...il.com,
        linux-amlogic@...ts.infradead.org, linux-clk@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v13 4/6] clk: meson: a1: add Amlogic A1 PLL clock
 controller driver

Hello Dmitry,

currently Jerome is busy so I am trying to continue where he left off.
I have followed the previous iterations a bit but may have missed some
details. So apologies if I'm repeating some questions that Jerome
previously asked.

On Wed, Apr 5, 2023 at 9:59 PM Dmitry Rokosov <ddrokosov@...rdevices.ru> wrote:
[...]
> +config COMMON_CLK_A1_PLL
> +       tristate "Meson A1 SoC PLL controller support"
Should this be "Amlogic A1 SoC PLL controller support"?
My understanding is that the "meson" name was dropped for this
generation of SoCs.

[...]
> +static const struct of_device_id a1_pll_clkc_match_table[] = {
> +       { .compatible = "amlogic,a1-pll-clkc", },
> +       {},
nit-pick: please drop the comma after {}
This empty entry is a sentinel, no other entries are supposed to come
after this - so a trailing comma is not necessary.

[...]
> +/* PLL register offset */
> +#define ANACTRL_FIXPLL_CTRL0   0x0
> +#define ANACTRL_FIXPLL_CTRL1   0x4
> +#define ANACTRL_FIXPLL_STS     0x14
> +#define ANACTRL_HIFIPLL_CTRL0  0xc0
> +#define ANACTRL_HIFIPLL_CTRL1  0xc4
> +#define ANACTRL_HIFIPLL_CTRL2  0xc8
> +#define ANACTRL_HIFIPLL_CTRL3  0xcc
> +#define ANACTRL_HIFIPLL_CTRL4  0xd0
> +#define ANACTRL_HIFIPLL_STS    0xd4
Here I have a question that will potentially affect patch 3/6
("dt-bindings: clock: meson: add A1 PLL clock controller bindings").
In the cover-letter you mentioned that quite a few clocks have been omitted.
Any dt-bindings that we create need to be stable going forward. That
means: the dt-bindings will always need to describe what the hardware
is capable of, not what the driver implements.
So my question is: do we have all needed inputs described in the
dt-bindings (even though we're omitting quite a few registers here
that will only be added/used in the future)?
Older SoCs require (temporarily) using the XTAL clock for CPU clock
tree changes. To make a long story short: I'm wondering if - at least
- the XTAL clock input is missing.

PS: I don't have an A1 datasheet nor a vendor kernel source (and even
less a board for testing). So I can't verify any of this myself and
I'm asking questions instead.


Best regards,
Martin