Quoting Jeffrey Hugo (2019-10-01 18:16:40) > diff --git a/drivers/clk/qcom/gpucc-msm8998.c b/drivers/clk/qcom/gpucc-msm8998.c > new file mode 100644 > index 000000000000..f0ccb4963885 > --- /dev/null > +++ b/drivers/clk/qcom/gpucc-msm8998.c > @@ -0,0 +1,346 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2019, Jeffrey Hugo > + */ > + > +#include <linux/kernel.h> > +#include <linux/bitops.h> > +#include <linux/err.h> > +#include <linux/platform_device.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/clk-provider.h> > +#include <linux/regmap.h> > +#include <linux/reset-controller.h> > +#include <linux/clk.h> Drop this include please. > + > + > +static struct clk_rcg2 rbcpr_clk_src = { > + .cmd_rcgr = 0x1030, > + .hid_width = 5, > + .parent_map = gpu_xo_gpll0_map, > + .freq_tbl = ftbl_rbcpr_clk_src, > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "rbcpr_clk_src", > + .parent_data = gpu_xo_gpll0, > + .num_parents = 2, > + .ops = &clk_rcg2_ops, > + }, > +}; > + > +static const struct freq_tbl ftbl_gfx3d_clk_src[] = { > + F(180000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0), > + F(257000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0), > + F(342000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0), > + F(414000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0), > + F(515000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0), > + F(596000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0), > + F(670000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0), > + F(710000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0), > + { } I guess this one doesn't do PLL ping pong? Instead we just reprogram the PLL all the time? Can we have rcg2 clk ops that set the rate on the parent to be exactly twice as much as the incoming frequency? I thought we already had this support in the code. Indeed, it is part of _freq_tbl_determine_rate() in clk-rcg.c, but not yet implemented in the same function name in clk-rcg2.c! Can you implement it? That way we don't need this long frequency table, just this weird one where it looks like: { .src = P_GPUPLL0_OUT_EVEN, .pre_div = 3 } { } And then some more logic in the rcg2 ops to allow this possibility for a frequency table when CLK_SET_RATE_PARENT is set. > +}; > + > +static struct clk_rcg2 gfx3d_clk_src = { > + .cmd_rcgr = 0x1070, > + .hid_width = 5, > + .parent_map = gpu_xo_gpupll0_map, > + .freq_tbl = ftbl_gfx3d_clk_src, > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "gfx3d_clk_src", > + .parent_data = gpu_xo_gpupll0, > + .num_parents = 2, > + .ops = &clk_rcg2_ops, > + .flags = CLK_OPS_PARENT_ENABLE, Needs CLK_SET_RATE_PARENT presumably? > + }, > +}; > + > +static const struct freq_tbl ftbl_rbbmtimer_clk_src[] = { > + F(19200000, P_XO, 1, 0, 0), > + { } > +}; > + [...] > + > +static const struct qcom_cc_desc gpucc_msm8998_desc = { > + .config = &gpucc_msm8998_regmap_config, > + .clks = gpucc_msm8998_clocks, > + .num_clks = ARRAY_SIZE(gpucc_msm8998_clocks), > + .resets = gpucc_msm8998_resets, > + .num_resets = ARRAY_SIZE(gpucc_msm8998_resets), > + .gdscs = gpucc_msm8998_gdscs, > + .num_gdscs = ARRAY_SIZE(gpucc_msm8998_gdscs), > +}; > + > +static const struct of_device_id gpucc_msm8998_match_table[] = { > + { .compatible = "qcom,gpucc-msm8998" }, The compatible is different. In the merged binding it is qcom,msm8998-gpucc. Either fix this or fix the binding please. > + { } > +}; > +MODULE_DEVICE_TABLE(of, gpucc_msm8998_match_table); > +