Hi Prabhakar, On Thu, Apr 23, 2020 at 11:41 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> wrote: > Add RZ/G1H (R8A7742) Clock Pulse Generator / Module Standby and Software > Reset support, using the CPG/MSSR driver core and the common R-Car Gen2 > (and RZ/G) code. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@xxxxxxxxxxxxxx> Thanks for your patch! > --- /dev/null > +++ b/drivers/clk/renesas/r8a7742-cpg-mssr.c > +static struct cpg_core_clk r8a7742_core_clks[] __initdata = { > + /* External Clock Inputs */ > + DEF_INPUT("extal", CLK_EXTAL), > + DEF_INPUT("usb_extal", CLK_USB_EXTAL), > + > + /* Internal Core Clocks */ > + DEF_BASE(".main", CLK_MAIN, CLK_TYPE_GEN2_MAIN, CLK_EXTAL), > + DEF_BASE(".pll0", CLK_PLL0, CLK_TYPE_GEN2_PLL0, CLK_MAIN), > + DEF_BASE(".pll1", CLK_PLL1, CLK_TYPE_GEN2_PLL1, CLK_MAIN), > + DEF_BASE(".pll3", CLK_PLL3, CLK_TYPE_GEN2_PLL3, CLK_MAIN), > + > + DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2, CLK_PLL1, 2, 1), > + > + /* Core Clock Outputs */ > + DEF_BASE("z", R8A7742_CLK_Z, CLK_TYPE_GEN2_Z, CLK_PLL0), > + DEF_BASE("sdh", R8A7742_CLK_SDH, CLK_TYPE_GEN2_SDH, CLK_PLL1), > + DEF_BASE("sd0", R8A7742_CLK_SD0, CLK_TYPE_GEN2_SD0, CLK_PLL1), > + DEF_BASE("sd1", R8A7742_CLK_SD1, CLK_TYPE_GEN2_SD1, CLK_PLL1), > + DEF_BASE("qspi", R8A7742_CLK_QSPI, CLK_TYPE_GEN2_QSPI, CLK_PLL1_DIV2), > + DEF_BASE("rcan", R8A7742_CLK_RCAN, CLK_TYPE_GEN2_RCAN, CLK_USB_EXTAL), > + > + DEF_FIXED("z2", R8A7742_CLK_Z2, CLK_PLL1, 2, 1), > + DEF_FIXED("zg", R8A7742_CLK_ZG, CLK_PLL1, 3, 1), > + DEF_FIXED("zt", R8A7742_CLK_ZT, CLK_PLL1, 5, 1), > + DEF_FIXED("ztr", R8A7742_CLK_ZTR, CLK_PLL1, 4, 1), > + DEF_FIXED("ztrd2", R8A7742_CLK_ZTRD2, CLK_PLL1, 12, 1), The ZT* clocks are not fixed-factor clocks, but use programmable dividers in FRQCRB. So either you implement them correctly, or you drop them, like we did for the other R-Car Gen2 and RZ/G1 SoCs (there are no users yet). > + DEF_FIXED("zx", R8A7742_CLK_ZX, CLK_PLL1, 3, 1), > + DEF_FIXED("zs", R8A7742_CLK_ZS, CLK_PLL1, 6, 1), > + DEF_FIXED("hp", R8A7742_CLK_HP, CLK_PLL1, 12, 1), > + DEF_FIXED("b", R8A7742_CLK_B, CLK_PLL1, 12, 1), > + DEF_FIXED("lb", R8A7742_CLK_LB, CLK_PLL1, 24, 1), Please use CLK_TYPE_GEN2_LB, as the LB divider depends on the state of mode pin MD18. > + DEF_FIXED("p", R8A7742_CLK_P, CLK_PLL1, 24, 1), > + DEF_FIXED("cl", R8A7742_CLK_CL, CLK_PLL1, 48, 1), > + DEF_FIXED("m2", R8A7742_CLK_M2, CLK_PLL1, 8, 1), > + DEF_FIXED("zb3", R8A7742_CLK_ZB3, CLK_PLL3, 4, 1), > + DEF_FIXED("zb3d2", R8A7742_CLK_ZB3D2, CLK_PLL3, 8, 1), > + DEF_FIXED("ddr", R8A7742_CLK_DDR, CLK_PLL3, 8, 1), > + DEF_FIXED("mp", R8A7742_CLK_MP, CLK_PLL1_DIV2, 15, 1), > + DEF_FIXED("cp", R8A7742_CLK_CP, CLK_EXTAL, 2, 1), > + DEF_FIXED("r", R8A7742_CLK_R, CLK_PLL1, 49152, 1), > + DEF_FIXED("osc", R8A7742_CLK_OSC, CLK_PLL1, 12288, 1), > + > + DEF_DIV6P1("sd2", R8A7742_CLK_SD2, CLK_PLL1_DIV2, 0x078), > + DEF_DIV6P1("sd3", R8A7742_CLK_SD3, CLK_PLL1_DIV2, 0x26c), > + DEF_DIV6P1("mmc0", R8A7742_CLK_MMC0, CLK_PLL1_DIV2, 0x240), > + DEF_DIV6P1("mmc1", R8A7742_CLK_MMC1, CLK_PLL1_DIV2, 0x244), > +}; > + > +static const struct mssr_mod_clk r8a7742_mod_clks[] __initconst = { > + DEF_MOD("msiof0", 0, R8A7742_CLK_MP), > + DEF_MOD("vcp1", 100, R8A7742_CLK_ZS), > + DEF_MOD("vcp0", 101, R8A7742_CLK_ZS), > + DEF_MOD("vpc1", 102, R8A7742_CLK_ZS), > + DEF_MOD("vpc0", 103, R8A7742_CLK_ZS), > + DEF_MOD("tmu1", 111, R8A7742_CLK_P), > + DEF_MOD("3dg", 112, R8A7742_CLK_ZG), > + DEF_MOD("2d-dmac", 115, R8A7742_CLK_ZS), > + DEF_MOD("fdp1-2", 117, R8A7742_CLK_ZS), > + DEF_MOD("fdp1-1", 118, R8A7742_CLK_ZS), > + DEF_MOD("fdp1-0", 119, R8A7742_CLK_ZS), > + DEF_MOD("tmu3", 121, R8A7742_CLK_P), > + DEF_MOD("tmu2", 122, R8A7742_CLK_P), > + DEF_MOD("cmt0", 124, R8A7742_CLK_R), > + DEF_MOD("tmu0", 125, R8A7742_CLK_CP), > + DEF_MOD("vsp1du1", 127, R8A7742_CLK_ZS), > + DEF_MOD("vsp1du0", 128, R8A7742_CLK_ZS), > + DEF_MOD("vsp1-sy", 131, R8A7742_CLK_ZS), > + DEF_MOD("scifa2", 202, R8A7742_CLK_MP), > + DEF_MOD("scifa1", 203, R8A7742_CLK_MP), > + DEF_MOD("scifa0", 204, R8A7742_CLK_MP), > + DEF_MOD("msiof2", 205, R8A7742_CLK_MP), > + DEF_MOD("scifb0", 206, R8A7742_CLK_MP), > + DEF_MOD("scifb1", 207, R8A7742_CLK_MP), > + DEF_MOD("msiof1", 208, R8A7742_CLK_MP), > + DEF_MOD("msiof3", 215, R8A7742_CLK_MP), > + DEF_MOD("scifb2", 216, R8A7742_CLK_MP), > + DEF_MOD("sys-dmac1", 218, R8A7742_CLK_ZS), > + DEF_MOD("sys-dmac0", 219, R8A7742_CLK_ZS), > + DEF_MOD("iic2", 300, R8A7742_CLK_CP), Parent should be R8A7742_CLK_HP. > + DEF_MOD("tpu0", 304, R8A7742_CLK_CP), > + DEF_MOD("mmcif1", 305, R8A7742_CLK_MMC1), > + DEF_MOD("scif2", 310, R8A7742_CLK_CP), Parent should be R8A7742_CLK_P. > + DEF_MOD("sdhi3", 311, R8A7742_CLK_SD3), > + DEF_MOD("sdhi2", 312, R8A7742_CLK_SD2), > + DEF_MOD("sdhi1", 313, R8A7742_CLK_SD2), Parent should be R8A7742_CLK_SD1. > +static int __init r8a7742_cpg_mssr_init(struct device *dev) > +{ > + const struct rcar_gen2_cpg_pll_config *cpg_pll_config; > + struct device_node *np = dev->of_node; > + unsigned int i; > + u32 cpg_mode; > + int error; > + > + error = rcar_rst_read_mode_pins(&cpg_mode); > + if (error) > + return error; > + > + cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)]; > + > + if (of_device_is_compatible(np, "renesas,r8a7742-cpg-mssr")) { > + /* RZ/G1H uses a 1/3 divider for ZG */ > + for (i = 0; i < ARRAY_SIZE(r8a7742_core_clks); i++) > + if (r8a7742_core_clks[i].id == R8A7742_CLK_ZG) { > + r8a7742_core_clks[i].div = 3; > + break; > + } > + } Do you really need this part? (copied from r8a7743-cpg-mssr.c ;-) If you remove it, r8a7742_core_clks[] can be const, and <linux/of.h> is no longer needed, > + return rcar_gen2_cpg_init(cpg_pll_config, 2, cpg_mode); > +} Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds