lkml.org 
[lkml]   [2016]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v2 6/6] clk: mvebu: Add the peripheral clock driver for Armada 3700
    Date
    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

    \
     
     \ /
      Last update: 2016-07-08 21:01    [W:4.336 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site