Quoting Gregory CLEMENT (2016-07-07 15:37:51) > +#include <linux/clk-provider.h> > +#include <linux/clk.h> Same question as my previous email. Is clk.h necessary? Is this driver also a clk consumer? > +static int armada_3700_add_composite_clk(const struct clk_periph_data *data, > + const char * const *parent_name, > + void __iomem *reg, spinlock_t *lock, > + struct device *dev, struct clk_hw *hw) > +{ > + const struct clk_ops *mux_ops = NULL, *gate_ops = NULL, > + *div_ops = NULL; > + struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *div_hw = NULL; > + const char * const *names; > + struct clk_mux *mux = NULL; > + struct clk_gate *gate = NULL; > + struct clk_divider *div = NULL; > + struct clk_double_div *double_div = NULL; > + int num_parent; > + int ret = 0; > + > + if (data->gate_shift != UNUSED) { > + gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL); > + > + if (!gate) > + return -ENOMEM; > + > + gate->reg = reg + CLK_DIS; > + gate->bit_idx = data->gate_shift; > + gate->lock = lock; > + gate_ops = &clk_gate_ops; > + gate_hw = &gate->hw; > + } > + > + if (data->mux_shift != UNUSED) { > + mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL); > + > + if (!mux) { > + ret = -ENOMEM; > + goto free_gate; > + } > + > + mux->reg = reg + TBG_SEL; > + mux->shift = data->mux_shift; > + mux->mask = 0x3; > + mux->lock = lock; > + mux_ops = &clk_mux_ro_ops; > + mux_hw = &mux->hw; > + } > + > + if (data->div_reg1 != UNUSED) { > + if (data->div_reg2 == UNUSED) { > + const struct clk_div_table *clkt; > + int table_size = 0; > + > + div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL); > + if (!div) { > + ret = -ENOMEM; > + goto free_mux; > + } > + > + div->reg = reg + data->div_reg1; > + div->table = data->table; > + for (clkt = div->table; clkt->div; clkt++) > + table_size++; > + div->width = order_base_2(table_size); > + div->lock = lock; > + div_ops = &clk_divider_ro_ops; > + div_hw = &div->hw; > + } else { > + double_div = devm_kzalloc(dev, sizeof(*double_div), > + GFP_KERNEL); > + if (!double_div) { > + ret = -ENOMEM; > + goto free_mux; > + } > + > + double_div->reg1 = reg + data->div_reg1; > + double_div->shift1 = data->div_shift1; > + double_div->reg2 = reg + data->div_reg1; > + double_div->shift2 = data->div_shift2; > + div_ops = &clk_double_div_ops; > + div_hw = &double_div->hw; > + } > + } > + > + switch (data->flags) { > + case XTAL_CHILD: > + /* the xtal clock is the 5th clock */ > + names = &parent_name[4]; > + num_parent = 1; > + break; > + case TBGA_S_CHILD: > + /* the TBG A S clock is the 3rd clock */ > + names = &parent_name[2]; > + num_parent = 1; > + break; > + case GBE_CORE_CHILD: > + names = &gbe_name[1]; > + num_parent = 1; > + break; > + case GBE_50_CHILD: > + names = &gbe_name[0]; > + num_parent = 1; > + break; > + case GBE_125_CHILD: > + names = &gbe_name[2]; > + num_parent = 1; > + break; > + default: > + names = parent_name; > + num_parent = 4; > + } > + hw = clk_hw_register_composite(dev, data->name, > + names, num_parent, > + mux_hw, mux_ops, > + div_hw, div_ops, > + gate_hw, gate_ops, > + CLK_IGNORE_UNUSED); > + if (IS_ERR(hw)) { > + ret = PTR_ERR(hw); > + goto free_div; > + } > + > + return 0; > +free_div: > + devm_kfree(dev, div); > + devm_kfree(dev, double_div); > +free_mux: > + devm_kfree(dev, mux); > +free_gate: > + devm_kfree(dev, gate); > + return ret; > +} Can this "add" function (aka registration function) be replaced with static data instead? I think that all of the static data exists already, this function can be removed and your probe can call clk_hw_register directly. It might need a macro though, since composite clock structures are rather messy. This avoids a lot of unnecessary allocations and time populating data that we already have access to. In general I am trying to encourage clk drivers to use only clk_hw_register() in their probe instead of the helper registration functions. Similarly I am discouraging drivers from populating hw.init at run-time, since we already have that data for that at compile-time. Regards, Mike -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html