[PATCH v13 4/6] clk: meson: a1: add Amlogic A1 PLL clock controller driver
Martin Blumenstingl
martin.blumenstingl at googlemail.com
Sun Apr 23 14:12:27 PDT 2023
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 at sberdevices.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
More information about the linux-amlogic
mailing list