On Tue, 18 Feb 2025 20:03:38 +0100
Jernej Škrabec <jernej.skrabec@xxxxxxxxx> wrote:
Hi Jernej,
many thanks for having a look!
> Dne petek, 14. februar 2025 ob 13:53:49 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> > Add the PLL clocks of the main CCU of the Allwinner A523 and T527 SoCs.
> > The clocks were modelled after the A523 and T527 manual, and double
> > checked by writing all 1's into the respective register, to spot all
> > implemented bits.
> >
> > The PLL and mod clocks for the two CPU clusters and the DSU are part of
> > a separate CCU, also most audio clocks are collected in a DSP CCU, so
> > both of these clock groups are missing from this driver.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> > ---
> > drivers/clk/sunxi-ng/Kconfig | 5 +
> > drivers/clk/sunxi-ng/Makefile | 2 +
> > drivers/clk/sunxi-ng/ccu-sun55i-a523.c | 481 +++++++++++++++++++++++++
> > drivers/clk/sunxi-ng/ccu-sun55i-a523.h | 14 +
> > drivers/clk/sunxi-ng/ccu_mp.h | 14 +-
> > 5 files changed, 510 insertions(+), 6 deletions(-)
> > create mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > create mode 100644 drivers/clk/sunxi-ng/ccu-sun55i-a523.h
> >
> > diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
> > index b547198a2c654..04efbda847cf9 100644
> > --- a/drivers/clk/sunxi-ng/Kconfig
> > +++ b/drivers/clk/sunxi-ng/Kconfig
> > @@ -52,6 +52,11 @@ config SUN50I_H6_R_CCU
> > default y
> > depends on ARM64 || COMPILE_TEST
> >
> > +config SUN55I_A523_CCU
> > + tristate "Support for the Allwinner A523/T527 CCU"
> > + default y
> > + depends on ARM64 || COMPILE_TEST
> > +
> > config SUN4I_A10_CCU
> > tristate "Support for the Allwinner A10/A20 CCU"
> > default y
> > diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
> > index 6b3ae2b620db6..01a887f7824bb 100644
> > --- a/drivers/clk/sunxi-ng/Makefile
> > +++ b/drivers/clk/sunxi-ng/Makefile
> > @@ -33,6 +33,7 @@ obj-$(CONFIG_SUN50I_A100_R_CCU) += sun50i-a100-r-ccu.o
> > obj-$(CONFIG_SUN50I_H6_CCU) += sun50i-h6-ccu.o
> > obj-$(CONFIG_SUN50I_H6_R_CCU) += sun50i-h6-r-ccu.o
> > obj-$(CONFIG_SUN50I_H616_CCU) += sun50i-h616-ccu.o
> > +obj-$(CONFIG_SUN55I_A523_CCU) += sun55i-a523-ccu.o
> > obj-$(CONFIG_SUN4I_A10_CCU) += sun4i-a10-ccu.o
> > obj-$(CONFIG_SUN5I_CCU) += sun5i-ccu.o
> > obj-$(CONFIG_SUN6I_A31_CCU) += sun6i-a31-ccu.o
> > @@ -58,6 +59,7 @@ sun50i-a100-r-ccu-y += ccu-sun50i-a100-r.o
> > sun50i-h6-ccu-y += ccu-sun50i-h6.o
> > sun50i-h6-r-ccu-y += ccu-sun50i-h6-r.o
> > sun50i-h616-ccu-y += ccu-sun50i-h616.o
> > +sun55i-a523-ccu-y += ccu-sun55i-a523.o
> > sun4i-a10-ccu-y += ccu-sun4i-a10.o
> > sun5i-ccu-y += ccu-sun5i.o
> > sun6i-a31-ccu-y += ccu-sun6i-a31.o
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun55i-a523.c b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > new file mode 100644
> > index 0000000000000..8374e841e9d82
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu-sun55i-a523.c
> > @@ -0,0 +1,481 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2023-2024 Arm Ltd.
> > + * Based on the D1 CCU driver:
> > + * Copyright (c) 2020 huangzhenwei@xxxxxxxxxxxxxxxxx
> > + * Copyright (C) 2021 Samuel Holland <samuel@xxxxxxxxxxxx>
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "../clk.h"
> > +
> > +#include "ccu_common.h"
> > +#include "ccu_reset.h"
> > +
> > +#include "ccu_div.h"
> > +#include "ccu_gate.h"
> > +#include "ccu_mp.h"
> > +#include "ccu_mult.h"
> > +#include "ccu_nk.h"
> > +#include "ccu_nkm.h"
> > +#include "ccu_nkmp.h"
> > +#include "ccu_nm.h"
> > +
> > +#include "ccu-sun55i-a523.h"
> > +
> > +/*
> > + * The 24 MHz oscillator, the root of most of the clock tree.
> > + * .fw_name is the string used in the DT "clock-names" property, used to
> > + * identify the corresponding clock in the "clocks" property.
> > + */
> > +static const struct clk_parent_data osc24M[] = {
> > + { .fw_name = "hosc" }
> > +};
> > +
> > +/**************************************************************************
> > + * PLLs *
> > + **************************************************************************/
> > +
> > +/* Some PLLs are input * N / div1 / P. Model them as NKMP with no K */
> > +#define SUN55I_A523_PLL_DDR0_REG 0x010
> > +static struct ccu_nkmp pll_ddr0_clk = {
> > + .enable = BIT(27),
> > + .lock = BIT(28),
> > + .n = _SUNXI_CCU_MULT_MIN(8, 8, 11),
> > + .m = _SUNXI_CCU_DIV(1, 1), /* input divider */
>
> Newer manuals, for example A523 manual v1.4, don't mention input dividers anymore.
> Newer BSP driver doesn't have them either. Should we drop them (for all PLLs)?
I don't know, as you figured, I found them in my copy of the manual. For
the PLLs we need today (PLL_PERIPH0) it shouldn't matter anyway: it's
programmed already, and AFAICS Linux actually never reprograms it(?).
In any case, I just left it in for now: the bit is definitely there, I
verified that in U-Boot by only seeing half of the MMC performance with
bit 1 set. We can remove them anytime later, without issue, can't we?
Actually, thinking about that: the manual pretty clearly says that for
instance PLL_PERIPH0 should be set to 1.2GHz and never changed or even
touched. I wonder if we should honour this somehow in the code? It seems
like the CCF doesn't do this anyway at the moment, but it could, I guess?
>
> > + .p = _SUNXI_CCU_DIV(0, 1), /* output divider */
> > + .common = {
> > + .reg = 0x010,
> > + .hw.init = CLK_HW_INIT_PARENTS_DATA("pll-ddr0", osc24M,
> > + &ccu_nkmp_ops,
> > + CLK_SET_RATE_GATE |
> > + CLK_IS_CRITICAL),
> > + },
> > +};
> > +
....
> > diff --git a/drivers/clk/sunxi-ng/ccu_mp.h b/drivers/clk/sunxi-ng/ccu_mp.h
> > index 687bd2ec798e2..5311835a4db60 100644
> > --- a/drivers/clk/sunxi-ng/ccu_mp.h
> > +++ b/drivers/clk/sunxi-ng/ccu_mp.h
> > @@ -100,20 +100,22 @@ struct ccu_mp {
>
> These changes doesn't belong in this commit.
Ah, indeed, I missed that. Moved that out into another patch now.
Cheers,
Andre
> Other than that, this looks like a good start. Thanks!
>
> Best regards,
> Jernej
>
> > _muxshift, _muxwidth, \
> > 0, _flags)
> >
> > -#define SUNXI_CCU_MP_DATA_WITH_MUX_GATE_FEAT(_struct, _name, _parents, _reg, \
> > +#define SUNXI_CCU_MP_MUX_GATE_POSTDIV_FEAT(_struct, _name, _parents, _reg, \
> > _mshift, _mwidth, \
> > _pshift, _pwidth, \
> > _muxshift, _muxwidth, \
> > - _gate, _flags, \
> > - _features) \
> > + _gate, _postdiv, \
> > + _flags, _features) \
> > struct ccu_mp _struct = { \
> > .enable = _gate, \
> > .m = _SUNXI_CCU_DIV(_mshift, _mwidth), \
> > .p = _SUNXI_CCU_DIV(_pshift, _pwidth), \
> > .mux = _SUNXI_CCU_MUX(_muxshift, _muxwidth), \
> > + .fixed_post_div = _postdiv, \
> > .common = { \
> > .reg = _reg, \
> > - .features = _features, \
> > + .features = CCU_FEATURE_FIXED_POSTDIV | \
> > + _features, \
> > .hw.init = CLK_HW_INIT_PARENTS_DATA(_name, \
> > _parents, \
> > &ccu_mp_ops,\
> > @@ -126,11 +128,11 @@ struct ccu_mp {
> > _pshift, _pwidth, \
> > _muxshift, _muxwidth, \
> > _gate, _flags) \
> > - SUNXI_CCU_MP_DATA_WITH_MUX_GATE_FEAT(_struct, _name, _parents, \
> > + SUNXI_CCU_MP_MUX_GATE_POSTDIV_FEAT(_struct, _name, _parents, \
> > _reg, _mshift, _mwidth, \
> > _pshift, _pwidth, \
> > _muxshift, _muxwidth, \
> > - _gate, _flags, 0)
> > + _gate, 1, _flags, 0)
> >
> > #define SUNXI_CCU_MP_DATA_WITH_MUX(_struct, _name, _parents, _reg, \
> > _mshift, _mwidth, \
> >
>
>
>
>
