Messages in this thread | | | From | Martin Blumenstingl <> | Date | Sun, 23 Apr 2023 23:12:27 +0200 | 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@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
| |