Re: [PATCHv9 13/43] clk: ti: add support for basic mux clock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/01/2013 11:01 PM, Nishanth Menon wrote:
On 10/25/2013 10:57 AM, Tero Kristo wrote:
[...]
diff --git a/drivers/clk/ti/mux.c b/drivers/clk/ti/mux.c
new file mode 100644
index 0000000..9c5259a
--- /dev/null
+++ b/drivers/clk/ti/mux.c
[...]
+/**
+ * of_mux_clk_setup() - Setup function for simple mux rate clock
+ */
+static int of_mux_clk_setup(struct device_node *node, struct regmap *regmap)

$ ./scripts/kernel-doc drivers/clk/ti/mux.c >/dev/null
Warning(drivers/clk/ti/mux.c:29): No description found for parameter
'node'
Warning(drivers/clk/ti/mux.c:29): No description found for parameter
'regmap'

I suggest in the next rev we do a verification if we have kernel doc
errors as well..

+{
+	struct clk *clk;
+	const char *clk_name = node->name;
+	void __iomem *reg;
+	int num_parents;
+	const char **parent_names;
+	int i;
+	u8 clk_mux_flags = 0;
+	u32 mask = 0;
+	u32 shift = 0;
+	u32 flags = 0;
+	u32 val;
+
+	num_parents = of_clk_get_parent_count(node);
+	if (num_parents < 1) {
+		pr_err("%s: mux-clock %s must have parent(s)\n",
+		       __func__, node->name);
+		return -EINVAL;
+	}
+	parent_names = kzalloc((sizeof(char *) * num_parents), GFP_KERNEL);
+	if (!parent_names) {
+		pr_err("%s: memory alloc failed\n", __func__);

as discussed, could be dropped.

Yep.


+		return -ENOMEM;
+	}
+
+	for (i = 0; i < num_parents; i++)
+		parent_names[i] = of_clk_get_parent_name(node, i);
+
+	of_property_read_u32(node, "reg", &val);

is'nt this mandatory? error check?

Will add.


+	reg = (void *)val;
+
+	if (of_property_read_u32(node, "ti,bit-shift", &shift)) {
+		pr_debug("%s: bit-shift property defaults to 0x%x for %s\n",
+			 __func__, shift, node->name);

why a debug if this is optional?

Well, it might be good for debugging if someone forgets the shift when he was supposed to add one. I could also make this a required property, but this will increase the dtb size slightly, there are quite a few mux-clocks around with 0 bit-shift atm.


+	}
+
+	if (of_property_read_bool(node, "ti,index-starts-at-one"))
+		clk_mux_flags |= CLK_MUX_INDEX_ONE;
+
+	if (of_property_read_bool(node, "ti,set-rate-parent"))
+		flags |= CLK_SET_RATE_PARENT;
+
+	/* Generate bit-mask based on parent info */
+	mask = num_parents;
+	if (!(clk_mux_flags & CLK_MUX_INDEX_ONE))
+		mask--;

we are assuming there wont be holes in the map (like reserved mux option?)

Yes. Currently this is the case at least, but we can add more beef to the binding to handle holes if this comes up in future. Or alternatively just add dummy-ck as mux parent and cover the holes that way.


+
+	mask = (1 << fls(mask)) - 1;
+
+	clk = clk_register_mux_table_regmap(NULL, clk_name, parent_names,
+					    num_parents, flags, reg, regmap,
+					    shift, mask, clk_mux_flags, NULL,
+					    NULL);
+
+	if (!IS_ERR(clk)) {
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+		return 0;
+	}
+

kfree(parent_names)?

Yea, will add.


+	return PTR_ERR(clk);
+}
+CLK_OF_DECLARE(mux_clk, "ti,mux-clock", of_mux_clk_setup);
+
+static int __init of_ti_composite_mux_clk_setup(struct device_node *node,
+						struct regmap *regmap)
+{
+	struct clk_mux *mux;
+	int num_parents;
+	int ret;
+	u32 val;
+
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return -ENOMEM;
+
+	of_property_read_u32(node, "reg", &val);
is'nt this mandatory? error check?

Will add.


+
+	mux->reg = (void *)val;
+	mux->regmap = regmap;
+
+	if (of_property_read_u32(node, "ti,bit-shift", &val)) {
+		pr_debug("%s: no bit-shift for %s, default=0\n",
+			 __func__, node->name);
+		val = 0;
+	}
+	mux->shift = val;
+
+	num_parents = of_clk_get_parent_count(node);

mandatory parameter without check?

Will add.


ti,index-starts-at-one, ti,set-rate-parent
these seem not supported here even though the bindings dont tell us that.

Yeah, composite-mux-clock documentation is somewhat broken at the moment, will look at that.


+
+	mux->mask = num_parents - 1;
+	mux->mask = (1 << fls(mux->mask)) - 1;
+
+	ret = ti_clk_add_component(node, &mux->hw, CLK_COMPONENT_TYPE_MUX);
+	if (!ret)
+		return 0;
+
+	kfree(mux);
+	return -ret;
+}
+CLK_OF_DECLARE(ti_composite_mux_clk_setup, "ti,composite-mux-clock",
+	       of_ti_composite_mux_clk_setup);




--
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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux