On Sun 23 May 16:10 CDT 2021, Martin Botka wrote: > From: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx> > > Add the clocks supported in global clock controller, which clock the > peripherals like BLSPs, SDCC, USB, MDSS etc. Register all the clocks > to the clock framework for the clients to be able to request for them. > > Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx> > Signed-off-by: Martin Botka <martin.botka@xxxxxxxxxxxxxx> This looks quite good to me, just two small things below. > diff --git a/drivers/clk/qcom/gcc-sm6125.c b/drivers/clk/qcom/gcc-sm6125.c [..] > +static struct clk_alpha_pll gpll0_out_early = { > + .offset = 0x0, > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT], > + .clkr = { > + .enable_reg = 0x79000, > + .enable_mask = BIT(0), > + .hw.init = &(struct clk_init_data){ > + .name = "gpll0_out_early", > + .parent_data = &(const struct clk_parent_data){ > + .fw_name = "bi_tcxo", > + .name = "bi_tcxo", For new drivers we don't need to rely on global name lookup, so just keep fw_name for the external clocks. > + }, > + .num_parents = 1, > + .ops = &clk_alpha_pll_ops, > + }, > + }, > +}; > + > +static struct clk_fixed_factor gpll0_out_aux2 = { > + .mult = 1, > + .div = 2, > + .hw.init = &(struct clk_init_data){ > + .name = "gpll0_out_aux2", > + .parent_data = &(const struct clk_parent_data){ > + .hw = &gpll0_out_early.clkr.hw, > + }, > + .num_parents = 1, > + .ops = &clk_fixed_factor_ops, > + }, > +}; > + > +static struct clk_fixed_factor gpll0_out_main = { > + .mult = 1, > + .div = 2, > + .hw.init = &(struct clk_init_data){ > + .name = "gpll0_out_main", > + .parent_data = &(const struct clk_parent_data){ Please use parent_hws instead when referencing a single hw in the same driver. > + .hw = &gpll0_out_early.clkr.hw, > + }, > + .num_parents = 1, > + .ops = &clk_fixed_factor_ops, > + }, > +}; > + Regards, Bjorn