On Mon 16 Jan 2023 at 15:42, Yu Tu <yu.tu@xxxxxxxxxxx> wrote: > Add the S4 PLL clock controller driver in the s4 SoC family. > > Signed-off-by: Yu Tu <yu.tu@xxxxxxxxxxx> > --- [...] > + > +static struct clk_regmap s4_fclk_div2 = { > + .data = &(struct clk_regmap_gate_data){ > + .offset = ANACTRL_FIXPLL_CTRL1, > + .bit_idx = 24, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "fclk_div2", > + .ops = &clk_regmap_gate_ro_ops, On the previous SoC, these fixed divider gate were not read-only. They are marked as critical when necessary, with the appropriate comment. Why is it different on the s4 ? > + .parent_hws = (const struct clk_hw *[]) { > + &s4_fclk_div2_div.hw > + }, > + .num_parents = 1, > + }, > +}; > + [...] > +#ifndef __MESON_S4_PLL_H__ > +#define __MESON_S4_PLL_H__ > + > +/* ANA_CTRL - Registers > + * REG_BASE: REGISTER_BASE_ADDR = 0xfe008000 This multi-line comment style is wrong in clk/ REG_BASE is not used so I'm not sure this is useful > + */ > +#define ANACTRL_FIXPLL_CTRL0 0x040 > +#define ANACTRL_FIXPLL_CTRL1 0x044 > +#define ANACTRL_FIXPLL_CTRL2 0x048 > +#define ANACTRL_FIXPLL_CTRL3 0x04c > +#define ANACTRL_FIXPLL_CTRL4 0x050 > +#define ANACTRL_FIXPLL_CTRL5 0x054 > +#define ANACTRL_FIXPLL_CTRL6 0x058 > +#define ANACTRL_FIXPLL_STS 0x05c > +#define ANACTRL_GP0PLL_CTRL0 0x080 > +#define ANACTRL_GP0PLL_CTRL1 0x084 > +#define ANACTRL_GP0PLL_CTRL2 0x088 > +#define ANACTRL_GP0PLL_CTRL3 0x08c > +#define ANACTRL_GP0PLL_CTRL4 0x090 > +#define ANACTRL_GP0PLL_CTRL5 0x094 > +#define ANACTRL_GP0PLL_CTRL6 0x098 > +#define ANACTRL_GP0PLL_STS 0x09c > +#define ANACTRL_HIFIPLL_CTRL0 0x100 > +#define ANACTRL_HIFIPLL_CTRL1 0x104 > +#define ANACTRL_HIFIPLL_CTRL2 0x108 > +#define ANACTRL_HIFIPLL_CTRL3 0x10c > +#define ANACTRL_HIFIPLL_CTRL4 0x110 > +#define ANACTRL_HIFIPLL_CTRL5 0x114 > +#define ANACTRL_HIFIPLL_CTRL6 0x118 > +#define ANACTRL_HIFIPLL_STS 0x11c > +#define ANACTRL_MPLL_CTRL0 0x180 > +#define ANACTRL_MPLL_CTRL1 0x184 > +#define ANACTRL_MPLL_CTRL2 0x188 > +#define ANACTRL_MPLL_CTRL3 0x18c > +#define ANACTRL_MPLL_CTRL4 0x190 > +#define ANACTRL_MPLL_CTRL5 0x194 > +#define ANACTRL_MPLL_CTRL6 0x198 > +#define ANACTRL_MPLL_CTRL7 0x19c > +#define ANACTRL_MPLL_CTRL8 0x1a0 > +#define ANACTRL_MPLL_STS 0x1a4 > +#define ANACTRL_HDMIPLL_CTRL0 0x1c0 > +#define ANACTRL_HDMIPLL_CTRL1 0x1c4 > +#define ANACTRL_HDMIPLL_CTRL2 0x1c8 > +#define ANACTRL_HDMIPLL_CTRL3 0x1cc > +#define ANACTRL_HDMIPLL_CTRL4 0x1d0 > +#define ANACTRL_HDMIPLL_CTRL5 0x1d4 > +#define ANACTRL_HDMIPLL_CTRL6 0x1d8 > +#define ANACTRL_HDMIPLL_STS 0x1dc > +#define ANACTRL_HDMIPLL_VLOCK 0x1e4 > + > +/* > + * CLKID index values > + * > + * These indices are entirely contrived and do not map onto the hardware. > + * It has now been decided to expose everything by default in the DT header: > + * include/dt-bindings/clock/axg-clkc.h. Only the clocks ids we don't want > + * to expose, such as the internal muxes and dividers of composite clocks, > + * will remain defined here. > + */ > +#define CLKID_FIXED_PLL_DCO 0 > +#define CLKID_FCLK_DIV2_DIV 2 > +#define CLKID_FCLK_DIV3_DIV 4 > +#define CLKID_FCLK_DIV4_DIV 6 > +#define CLKID_FCLK_DIV5_DIV 8 > +#define CLKID_FCLK_DIV7_DIV 10 > +#define CLKID_FCLK_DIV2P5_DIV 12 > +#define CLKID_GP0_PLL_DCO 14 > +#define CLKID_HIFI_PLL_DCO 16 > +#define CLKID_HDMI_PLL_DCO 18 > +#define CLKID_HDMI_PLL_OD 19 > +#define CLKID_MPLL_50M_DIV 21 > +#define CLKID_MPLL_PREDIV 23 > +#define CLKID_MPLL0_DIV 24 > +#define CLKID_MPLL1_DIV 26 > +#define CLKID_MPLL2_DIV 28 > +#define CLKID_MPLL3_DIV 30 > + > +#define NR_PLL_CLKS 32 > +/* include the CLKIDs that have been made part of the DT binding */ > +#include <dt-bindings/clock/amlogic,s4-pll-clkc.h> > + > +#endif /* __MESON_S4_PLL_H__ */