[PATCH 1/2] clk: Add composite clock type

[PATCH 1/2] clk: Add composite clock type

[Prashant Gaikwad]

Not all clocks are required to be decomposed into basic clock
types but at the same time want to use the functionality
provided by these basic clock types instead of duplicating.

For example, Tegra SoC has ~100 clocks which can be decomposed
into Mux -> Div -> Gate clock types making the clock count to
~300. Also, parent change operation can not be performed on gate
clock which forces to use mux clock in driver if want to change
the parent.

Instead aggregate the basic clock types functionality into one
clock and just use this clock for all operations. This clock
type re-uses the functionality of basic clock types and not
limited to basic clock types but any hardware-specific
implementation.

Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com>
---
 drivers/clk/Makefile         |    3 +-
 drivers/clk/clk-composite.c  |  208 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h |   30 ++++++
 3 files changed, 240 insertions(+), 1 deletions(-)
 create mode 100644 drivers/clk/clk-composite.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index f0b269a..baf7608 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -2,7 +2,8 @@
 obj-$(CONFIG_HAVE_CLK)		+= clk-devres.o
 obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
 obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-fixed-rate.o clk-gate.o \
-				   clk-mux.o clk-divider.o clk-fixed-factor.o
+				   clk-mux.o clk-divider.o clk-fixed-factor.o \
+				   clk-composite.o
 # SoCs specific
 obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
 obj-$(CONFIG_ARCH_NOMADIK)	+= clk-nomadik.o
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
new file mode 100644
index 0000000..8634dbf
--- /dev/null
+++ b/drivers/clk/clk-composite.c
@@ -0,0 +1,208 @@
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+
+#define to_clk_composite(_hw) container_of(_hw, struct clk_composite, hw)
+
+static u8 clk_composite_get_parent(struct clk_hw *hw)
+{
+	struct clk_composite *composite = to_clk_composite(hw);
+	const struct clk_ops *mux_ops = composite->mux_ops;
+	struct clk_hw *mux_hw = composite->mux_hw;
+
+	mux_hw->clk = hw->clk;
+
+	return mux_ops->get_parent(mux_hw);
+}
+
+static int clk_composite_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct clk_composite *composite = to_clk_composite(hw);
+	const struct clk_ops *mux_ops = composite->mux_ops;
+	struct clk_hw *mux_hw = composite->mux_hw;
+
+	mux_hw->clk = hw->clk;
+
+	return mux_ops->set_parent(mux_hw, index);
+}
+
+static unsigned long clk_composite_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	struct clk_composite *composite = to_clk_composite(hw);
+	const struct clk_ops *div_ops = composite->div_ops;
+	struct clk_hw *div_hw = composite->div_hw;
+
+	div_hw->clk = hw->clk;
+
+	return div_ops->recalc_rate(div_hw, parent_rate);
+}
+
+static long clk_composite_round_rate(struct clk_hw *hw, unsigned long rate,
+				  unsigned long *prate)
+{
+	struct clk_composite *composite = to_clk_composite(hw);
+	const struct clk_ops *div_ops = composite->div_ops;
+	struct clk_hw *div_hw = composite->div_hw;
+
+	div_hw->clk = hw->clk;
+
+	return div_ops->round_rate(div_hw, rate, prate);
+}
+
+static int clk_composite_set_rate(struct clk_hw *hw, unsigned long rate,
+			       unsigned long parent_rate)
+{
+	struct clk_composite *composite = to_clk_composite(hw);
+	const struct clk_ops *div_ops = composite->div_ops;
+	struct clk_hw *div_hw = composite->div_hw;
+
+	div_hw->clk = hw->clk;
+
+	return div_ops->set_rate(div_hw, rate, parent_rate);
+}
+
+static int clk_composite_is_enabled(struct clk_hw *hw)
+{
+	struct clk_composite *composite = to_clk_composite(hw);
+	const struct clk_ops *gate_ops = composite->gate_ops;
+	struct clk_hw *gate_hw = composite->gate_hw;
+
+	gate_hw->clk = hw->clk;
+
+	return gate_ops->is_enabled(gate_hw);
+}
+
+static int clk_composite_enable(struct clk_hw *hw)
+{
+	struct clk_composite *composite = to_clk_composite(hw);
+	const struct clk_ops *gate_ops = composite->gate_ops;
+	struct clk_hw *gate_hw = composite->gate_hw;
+
+	gate_hw->clk = hw->clk;
+
+	return gate_ops->enable(gate_hw);
+}
+
+static void clk_composite_disable(struct clk_hw *hw)
+{
+	struct clk_composite *composite = to_clk_composite(hw);
+	const struct clk_ops *gate_ops = composite->gate_ops;
+	struct clk_hw *gate_hw = composite->gate_hw;
+
+	gate_hw->clk = hw->clk;
+
+	gate_ops->disable(gate_hw);
+}
+
+struct clk *clk_register_composite(struct device *dev, const char *name,
+			const char **parent_names, int num_parents,
+			struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+			struct clk_hw *div_hw, const struct clk_ops *div_ops,
+			struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+			unsigned long flags)
+{
+	struct clk *clk;
+	struct clk_init_data init;
+	struct clk_composite *composite;
+	struct clk_ops *clk_composite_ops;
+
+	composite = kzalloc(sizeof(struct clk_ops), GFP_KERNEL);
+	if (!composite) {
+		pr_err("%s: could not allocate composite clk\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	init.name = name;
+	init.flags = flags | CLK_IS_BASIC;
+	init.parent_names = parent_names;
+	init.num_parents = num_parents;
+
+	/* allocate the clock ops */
+	clk_composite_ops = kzalloc(sizeof(struct clk_ops), GFP_KERNEL);
+	if (!clk_composite_ops) {
+		pr_err("%s: could not allocate clk ops\n", __func__);
+		kfree(composite);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	if (mux_hw && mux_ops) {
+		if (!mux_ops->get_parent || !mux_ops->set_parent) {
+			clk = ERR_PTR(-EINVAL);
+			goto err;
+		}
+
+		composite->mux_hw = mux_hw;
+		composite->mux_ops = mux_ops;
+		clk_composite_ops->get_parent = clk_composite_get_parent;
+		clk_composite_ops->set_parent = clk_composite_set_parent;
+	}
+
+	if (div_hw && div_ops) {
+		if (!div_ops->recalc_rate || !div_ops->round_rate ||
+		    !div_ops->set_rate) {
+			clk = ERR_PTR(-EINVAL);
+			goto err;
+		}
+
+		composite->div_hw = div_hw;
+		composite->div_ops = div_ops;
+		clk_composite_ops->recalc_rate = clk_composite_recalc_rate;
+		clk_composite_ops->round_rate = clk_composite_round_rate;
+		clk_composite_ops->set_rate = clk_composite_set_rate;
+	}
+
+	if (gate_hw && gate_ops) {
+		if (!gate_ops->is_enabled || !gate_ops->enable ||
+		    !gate_ops->disable) {
+			clk = ERR_PTR(-EINVAL);
+			goto err;
+		}
+
+		composite->gate_hw = gate_hw;
+		composite->gate_ops = gate_ops;
+		clk_composite_ops->is_enabled = clk_composite_is_enabled;
+		clk_composite_ops->enable = clk_composite_enable;
+		clk_composite_ops->disable = clk_composite_disable;
+	}
+
+	init.ops = clk_composite_ops;
+	composite->hw.init = &init;
+
+	clk = clk_register(NULL, &composite->hw);
+	if (IS_ERR(clk))
+		goto err;
+
+	if (composite->mux_hw)
+		composite->mux_hw->clk = clk;
+
+	if (composite->div_hw)
+		composite->div_hw->clk = clk;
+
+	if (composite->gate_hw)
+		composite->gate_hw->clk = clk;
+
+	return clk;
+
+err:
+	kfree(clk_composite_ops);
+	kfree(composite);
+	return clk;
+}
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4989b8a..5950e9e 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -325,6 +325,36 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		unsigned int mult, unsigned int div);
 
+/***
+ * struct clk_composite - aggregate clock of mux, divider and gate clocks
+ *
+ * @hw:		handle between common and hardware-specific interfaces
+ * @mux_hw:	handle between composite and hardware-specifix mux clock
+ * @div_hw:	handle between composite and hardware-specifix divider clock
+ * @gate_hw:	handle between composite and hardware-specifix gate clock
+ * @mux_ops:	clock ops for mux
+ * @div_ops:	clock ops for divider
+ * @gate_ops:	clock ops for gate
+ */
+struct clk_composite {
+	struct clk_hw	hw;
+
+	struct clk_hw	*mux_hw;
+	struct clk_hw	*div_hw;
+	struct clk_hw	*gate_hw;
+
+	const struct clk_ops	*mux_ops;
+	const struct clk_ops	*div_ops;
+	const struct clk_ops	*gate_ops;
+};
+
+struct clk *clk_register_composite(struct device *dev, const char *name,
+		const char **parent_names, int num_parents,
+		struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
+		struct clk_hw *div_hw, const struct clk_ops *div_ops,
+		struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
+		unsigned long flags);
+
 /**
  * clk_register - allocate a new clock, register it and return an opaque cookie
  * @dev: device that is registering this clock
-- 
1.7.4.1

[PATCH 2/2] clk: tegra30: Convert clk out to composite clk

[Prashant Gaikwad]

Convert clk out to composite clock type which removes
the mux clock.

Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com>
---
This patch is rebased on ccf-rework for Tegra patch series. It is just to show
how clk-composite can be used, not to be merged. If patch 1 is accepted then
I would like to merge this patch to ccf-rework series.
---
 drivers/clk/tegra/clk-tegra30.c |   51 +++++++++++++-------------------------
 drivers/clk/tegra/clk.h         |   49 +++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 33 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 30fb743..4c16c11 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1191,43 +1191,28 @@ static void __init tegra30_audio_clk_init(void)
 	clks[spdif_2x] = clk;
 }
 
+static struct tegra_clk_out_init_data tegra_clk_out_list[] = {
+	TEGRA_CLK_OUT_INIT_DATA("clk_out_1", "extern1", "clk_out_1", clk_out1_parents, PMC_CLK_OUT_CNTRL, 6, 3, 0, 2, 0, &clk_out_lock, clk_out_1),
+	TEGRA_CLK_OUT_INIT_DATA("clk_out_2", "extern2", "clk_out_2", clk_out2_parents, PMC_CLK_OUT_CNTRL, 14, 3, 0, 10, 0, &clk_out_lock, clk_out_2),
+	TEGRA_CLK_OUT_INIT_DATA("clk_out_3", "extern3", "clk_out_3", clk_out3_parents, PMC_CLK_OUT_CNTRL, 22, 3, 0, 18, 0, &clk_out_lock, clk_out_3),
+};
+
 static void __init tegra30_pmc_clk_init(void)
 {
 	struct clk *clk;
+	int i;
 
-	/* clk_out_1 */
-	clk = clk_register_mux(NULL, "clk_out_1_mux", clk_out1_parents,
-			       ARRAY_SIZE(clk_out1_parents), 0,
-			       pmc_base + PMC_CLK_OUT_CNTRL, 6, 3, 0,
-			       &clk_out_lock);
-	clks[clk_out_1_mux] = clk;
-	clk = clk_register_gate(NULL, "clk_out_1", "clk_out_1_mux", 0,
-				pmc_base + PMC_CLK_OUT_CNTRL, 2, 0,
-				&clk_out_lock);
-	clk_register_clkdev(clk, "extern1", "clk_out_1");
-	clks[clk_out_1] = clk;
-
-	/* clk_out_2 */
-	clk = clk_register_mux(NULL, "clk_out_2_mux", clk_out2_parents,
-			       ARRAY_SIZE(clk_out1_parents), 0,
-			       pmc_base + PMC_CLK_OUT_CNTRL, 14, 3, 0,
-			       &clk_out_lock);
-	clk = clk_register_gate(NULL, "clk_out_2", "clk_out_2_mux", 0,
-				pmc_base + PMC_CLK_OUT_CNTRL, 10, 0,
-				&clk_out_lock);
-	clk_register_clkdev(clk, "extern2", "clk_out_2");
-	clks[clk_out_2] = clk;
-
-	/* clk_out_3 */
-	clk = clk_register_mux(NULL, "clk_out_3_mux", clk_out3_parents,
-			       ARRAY_SIZE(clk_out1_parents), 0,
-			       pmc_base + PMC_CLK_OUT_CNTRL, 22, 3, 0,
-			       &clk_out_lock);
-	clk = clk_register_gate(NULL, "clk_out_3", "clk_out_3_mux", 0,
-				pmc_base + PMC_CLK_OUT_CNTRL, 18, 0,
-				&clk_out_lock);
-	clk_register_clkdev(clk, "extern3", "clk_out_3");
-	clks[clk_out_3] = clk;
+	for (i = 0; i < ARRAY_SIZE(tegra_clk_out_list); i++) {
+		struct tegra_clk_out_init_data *out = &tegra_clk_out_list[i];
+
+		out->out.mux.reg = pmc_base + out->offset;
+		out->out.gate.reg = pmc_base + out->offset;
+
+		clk = clk_register_composite(NULL, out->name, out->parent_names,
+			out->num_parents, &out->out.mux.hw, &clk_mux_ops,
+			NULL, NULL, &out->out.gate.hw, &clk_gate_ops, 0);
+		clks[out->clk_id] = clk;
+	}
 
 	/* blink */
 	clk = clk_register_gate(NULL, "blink_override", "clk_32k", 0,
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index f1ed1d0..47c536d 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -437,6 +437,55 @@ struct clk *tegra_clk_super_mux(const char *name, const char **parent_names,
 				u8 width, u8 pllx_index, u8 div2_index,
 				spinlock_t *lock);
 
+struct tegra_clk_out {
+	struct clk_hw	hw;
+	struct clk_mux	mux;
+	struct clk_gate	gate;
+};
+
+#define TEGRA_CLK_OUT(_mux_shift, _mux_width, _mux_flags,	\
+			_gate_bit_idx, _gate_flags, _lock)	\
+	{							\
+		.mux = {					\
+			.shift = _mux_shift,			\
+			.width = _mux_width,			\
+			.flags = _mux_flags,			\
+			.lock = _lock,				\
+		},						\
+		.gate = {					\
+			.bit_idx = _gate_bit_idx,		\
+			.flags = _gate_flags,			\
+			.lock = _lock,				\
+		},						\
+	}
+
+struct tegra_clk_out_init_data {
+	const char *name;
+	int clk_id;
+	const char **parent_names;
+	int num_parents;
+	struct tegra_clk_out out;
+	u32 offset;
+	const char *con_id;
+	const char *dev_id;
+};
+
+#define TEGRA_CLK_OUT_INIT_DATA(_name, _con_id, _dev_id, _parent_names,	\
+			_offset, _mux_shift, _mux_width, _mux_flags,	\
+			_gate_bit_idx, _gate_flags, _lock, _clk_id)	\
+	{								\
+		.name = _name,						\
+		.clk_id = _clk_id,					\
+		.parent_names = _parent_names,				\
+		.num_parents = ARRAY_SIZE(_parent_names),		\
+		.out = TEGRA_CLK_OUT(_mux_shift, _mux_width,		\
+			 	     _mux_flags, _gate_bit_idx,		\
+				     _gate_flags, _lock),		\
+		.offset = _offset,					\
+		.con_id = _con_id,					\
+		.dev_id = _dev_id,					\
+	}
+
 /**
  * struct clk_init_tabel - clock initialization table
  * @clk_id:	clock id as mentioned in device tree bindings
-- 
1.7.4.1

[PATCH 2/2] clk: tegra30: Convert clk out to composite clk

[Stephen Warren]

On 01/03/2013 10:51 PM, Prashant Gaikwad wrote:
> Convert clk out to composite clock type which removes
> the mux clock.
> 
> Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com>
> ---
> This patch is rebased on ccf-rework for Tegra patch series. It is just to show
> how clk-composite can be used, not to be merged. If patch 1 is accepted then
> I would like to merge this patch to ccf-rework series.

Just so I'm clear, is the intent that patch 1 of this series gets
reviewed/accepted, and then you'll repost an updated version of the
Tegra CCF rework series that relies on patch 1? If so, patch 1 would
need to be either taken through the Tegra tree, or put into a separate
branch in the clock tree, so the Tegra tree can merge it as a dependency
of the Tegra CCF rework branch.

[PATCH 1/2] clk: Add composite clock type

[Stephen Boyd]

On 01/03/13 21:51, Prashant Gaikwad wrote:
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index f0b269a..baf7608 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -2,7 +2,8 @@
>  obj-$(CONFIG_HAVE_CLK)		+= clk-devres.o
>  obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
>  obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-fixed-rate.o clk-gate.o \
> -				   clk-mux.o clk-divider.o clk-fixed-factor.o
> +				   clk-mux.o clk-divider.o clk-fixed-factor.o \
> +				   clk-composite.o

This list is getting a little out of hand. Should we sort it
alphabetically and put each file on one line?

>  # SoCs specific
>  obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
>  obj-$(CONFIG_ARCH_NOMADIK)	+= clk-nomadik.o
> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> new file mode 100644
> index 0000000..8634dbf
> --- /dev/null
> +++ b/drivers/clk/clk-composite.c
> @@ -0,0 +1,208 @@
> +/*
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +
> +#define to_clk_composite(_hw) container_of(_hw, struct clk_composite, hw)
> +
> +static u8 clk_composite_get_parent(struct clk_hw *hw)
> +{
> +	struct clk_composite *composite = to_clk_composite(hw);
> +	const struct clk_ops *mux_ops = composite->mux_ops;
> +	struct clk_hw *mux_hw = composite->mux_hw;
> +
> +	mux_hw->clk = hw->clk;

Looks like this is already done down in the register function. Why are
we doing it again here and in each op?

> +
> +	return mux_ops->get_parent(mux_hw);
> +}
[snip]
> +struct clk *clk_register_composite(struct device *dev, const char *name,
> +			const char **parent_names, int num_parents,
> +			struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
> +			struct clk_hw *div_hw, const struct clk_ops *div_ops,
> +			struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
> +			unsigned long flags)
> +{
> +	struct clk *clk;
> +	struct clk_init_data init;
> +	struct clk_composite *composite;
> +	struct clk_ops *clk_composite_ops;
> +
> +	composite = kzalloc(sizeof(struct clk_ops), GFP_KERNEL);

sizeof(*composite) != sizeof(struct clk_ops)

> +	if (!composite) {
> +		pr_err("%s: could not allocate composite clk\n", __func__);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	init.name = name;
> +	init.flags = flags | CLK_IS_BASIC;
> +	init.parent_names = parent_names;
> +	init.num_parents = num_parents;
> +
> +	/* allocate the clock ops */
> +	clk_composite_ops = kzalloc(sizeof(struct clk_ops), GFP_KERNEL);

This one looks right though. Perhaps you should change style to use
sizeof(*clk_composite_ops) so that the above mistake doesn't happen.

> +	if (!clk_composite_ops) {
> +		pr_err("%s: could not allocate clk ops\n", __func__);
> +		kfree(composite);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	if (mux_hw && mux_ops) {
> +		if (!mux_ops->get_parent || !mux_ops->set_parent) {
> +			clk = ERR_PTR(-EINVAL);
> +			goto err;
> +		}
> +
> +		composite->mux_hw = mux_hw;
> +		composite->mux_ops = mux_ops;
> +		clk_composite_ops->get_parent = clk_composite_get_parent;
> +		clk_composite_ops->set_parent = clk_composite_set_parent;
> +	}
> +
> +	if (div_hw && div_ops) {
> +		if (!div_ops->recalc_rate || !div_ops->round_rate ||
> +		    !div_ops->set_rate) {
> +			clk = ERR_PTR(-EINVAL);
> +			goto err;
> +		}
> +
> +		composite->div_hw = div_hw;
> +		composite->div_ops = div_ops;
> +		clk_composite_ops->recalc_rate = clk_composite_recalc_rate;
> +		clk_composite_ops->round_rate = clk_composite_round_rate;
> +		clk_composite_ops->set_rate = clk_composite_set_rate;
> +	}
> +
> +	if (gate_hw && gate_ops) {
> +		if (!gate_ops->is_enabled || !gate_ops->enable ||
> +		    !gate_ops->disable) {
> +			clk = ERR_PTR(-EINVAL);
> +			goto err;
> +		}
> +
> +		composite->gate_hw = gate_hw;
> +		composite->gate_ops = gate_ops;
> +		clk_composite_ops->is_enabled = clk_composite_is_enabled;
> +		clk_composite_ops->enable = clk_composite_enable;
> +		clk_composite_ops->disable = clk_composite_disable;
> +	}
> +
> +	init.ops = clk_composite_ops;
> +	composite->hw.init = &init;
> +
> +	clk = clk_register(NULL, &composite->hw);

Please pass dev here.

> +	if (IS_ERR(clk))
> +		goto err;
> +
> +	if (composite->mux_hw)
> +		composite->mux_hw->clk = clk;
> +
> +	if (composite->div_hw)
> +		composite->div_hw->clk = clk;
> +
> +	if (composite->gate_hw)
> +		composite->gate_hw->clk = clk;
> +
> +	return clk;
> +
> +err:
> +	kfree(clk_composite_ops);
> +	kfree(composite);
> +	return clk;
> +}
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 1/2] clk: Add composite clock type

[Prashant Gaikwad]

On Saturday 05 January 2013 03:48 AM, Stephen Boyd wrote:
> On 01/03/13 21:51, Prashant Gaikwad wrote:
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index f0b269a..baf7608 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -2,7 +2,8 @@
>>   obj-$(CONFIG_HAVE_CLK)		+= clk-devres.o
>>   obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
>>   obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-fixed-rate.o clk-gate.o \
>> -				   clk-mux.o clk-divider.o clk-fixed-factor.o
>> +				   clk-mux.o clk-divider.o clk-fixed-factor.o \
>> +				   clk-composite.o
> This list is getting a little out of hand. Should we sort it
> alphabetically and put each file on one line?

Do you want me to do it in this patch?

>
>>   # SoCs specific
>>   obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
>>   obj-$(CONFIG_ARCH_NOMADIK)	+= clk-nomadik.o
>> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
>> new file mode 100644
>> index 0000000..8634dbf
>> --- /dev/null
>> +++ b/drivers/clk/clk-composite.c
>> @@ -0,0 +1,208 @@
>> +/*
>> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/err.h>
>> +#include <linux/slab.h>
>> +
>> +#define to_clk_composite(_hw) container_of(_hw, struct clk_composite, hw)
>> +
>> +static u8 clk_composite_get_parent(struct clk_hw *hw)
>> +{
>> +	struct clk_composite *composite = to_clk_composite(hw);
>> +	const struct clk_ops *mux_ops = composite->mux_ops;
>> +	struct clk_hw *mux_hw = composite->mux_hw;
>> +
>> +	mux_hw->clk = hw->clk;
> Looks like this is already done down in the register function. Why are
> we doing it again here and in each op?

Some ops gets called during clk_init which is before clk_register returns.


>> +
>> +	return mux_ops->get_parent(mux_hw);
>> +}
> [snip]
>> +struct clk *clk_register_composite(struct device *dev, const char *name,
>> +			const char **parent_names, int num_parents,
>> +			struct clk_hw *mux_hw, const struct clk_ops *mux_ops,
>> +			struct clk_hw *div_hw, const struct clk_ops *div_ops,
>> +			struct clk_hw *gate_hw, const struct clk_ops *gate_ops,
>> +			unsigned long flags)
>> +{
>> +	struct clk *clk;
>> +	struct clk_init_data init;
>> +	struct clk_composite *composite;
>> +	struct clk_ops *clk_composite_ops;
>> +
>> +	composite = kzalloc(sizeof(struct clk_ops), GFP_KERNEL);
> sizeof(*composite) != sizeof(struct clk_ops)

Thanks.

>> +	if (!composite) {
>> +		pr_err("%s: could not allocate composite clk\n", __func__);
>> +		return ERR_PTR(-ENOMEM);
>> +	}
>> +
>> +	init.name = name;
>> +	init.flags = flags | CLK_IS_BASIC;
>> +	init.parent_names = parent_names;
>> +	init.num_parents = num_parents;
>> +
>> +	/* allocate the clock ops */
>> +	clk_composite_ops = kzalloc(sizeof(struct clk_ops), GFP_KERNEL);
> This one looks right though. Perhaps you should change style to use
> sizeof(*clk_composite_ops) so that the above mistake doesn't happen.

Sure.

>> +	if (!clk_composite_ops) {
>> +		pr_err("%s: could not allocate clk ops\n", __func__);
>> +		kfree(composite);
>> +		return ERR_PTR(-ENOMEM);
>> +	}
>> +
>> +	if (mux_hw && mux_ops) {
>> +		if (!mux_ops->get_parent || !mux_ops->set_parent) {
>> +			clk = ERR_PTR(-EINVAL);
>> +			goto err;
>> +		}
>> +
>> +		composite->mux_hw = mux_hw;
>> +		composite->mux_ops = mux_ops;
>> +		clk_composite_ops->get_parent = clk_composite_get_parent;
>> +		clk_composite_ops->set_parent = clk_composite_set_parent;
>> +	}
>> +
>> +	if (div_hw && div_ops) {
>> +		if (!div_ops->recalc_rate || !div_ops->round_rate ||
>> +		    !div_ops->set_rate) {
>> +			clk = ERR_PTR(-EINVAL);
>> +			goto err;
>> +		}
>> +
>> +		composite->div_hw = div_hw;
>> +		composite->div_ops = div_ops;
>> +		clk_composite_ops->recalc_rate = clk_composite_recalc_rate;
>> +		clk_composite_ops->round_rate = clk_composite_round_rate;
>> +		clk_composite_ops->set_rate = clk_composite_set_rate;
>> +	}
>> +
>> +	if (gate_hw && gate_ops) {
>> +		if (!gate_ops->is_enabled || !gate_ops->enable ||
>> +		    !gate_ops->disable) {
>> +			clk = ERR_PTR(-EINVAL);
>> +			goto err;
>> +		}
>> +
>> +		composite->gate_hw = gate_hw;
>> +		composite->gate_ops = gate_ops;
>> +		clk_composite_ops->is_enabled = clk_composite_is_enabled;
>> +		clk_composite_ops->enable = clk_composite_enable;
>> +		clk_composite_ops->disable = clk_composite_disable;
>> +	}
>> +
>> +	init.ops = clk_composite_ops;
>> +	composite->hw.init = &init;
>> +
>> +	clk = clk_register(NULL, &composite->hw);
> Please pass dev here.

Yes.

>> +	if (IS_ERR(clk))
>> +		goto err;
>> +
>> +	if (composite->mux_hw)
>> +		composite->mux_hw->clk = clk;
>> +
>> +	if (composite->div_hw)
>> +		composite->div_hw->clk = clk;
>> +
>> +	if (composite->gate_hw)
>> +		composite->gate_hw->clk = clk;
>> +
>> +	return clk;
>> +
>> +err:
>> +	kfree(clk_composite_ops);
>> +	kfree(composite);
>> +	return clk;
>> +}
>>

[PATCH 2/2] clk: tegra30: Convert clk out to composite clk

[Prashant Gaikwad]

On Friday 04 January 2013 09:55 PM, Stephen Warren wrote:
> On 01/03/2013 10:51 PM, Prashant Gaikwad wrote:
>> Convert clk out to composite clock type which removes
>> the mux clock.
>>
>> Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com>
>> ---
>> This patch is rebased on ccf-rework for Tegra patch series. It is just to show
>> how clk-composite can be used, not to be merged. If patch 1 is accepted then
>> I would like to merge this patch to ccf-rework series.
> Just so I'm clear, is the intent that patch 1 of this series gets
> reviewed/accepted, and then you'll repost an updated version of the
> Tegra CCF rework series that relies on patch 1? If so, patch 1 would
> need to be either taken through the Tegra tree, or put into a separate
> branch in the clock tree, so the Tegra tree can merge it as a dependency
> of the Tegra CCF rework branch.

Yes, that is my plan but you can tell whatever you are comfortable with. 
I will re-order the dependencies.

[PATCH 1/2] clk: Add composite clock type

[Stephen Boyd]

On 01/04/13 18:49, Prashant Gaikwad wrote:
> On Saturday 05 January 2013 03:48 AM, Stephen Boyd wrote:
>> On 01/03/13 21:51, Prashant Gaikwad wrote:
>>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>>> index f0b269a..baf7608 100644
>>> --- a/drivers/clk/Makefile
>>> +++ b/drivers/clk/Makefile
>>> @@ -2,7 +2,8 @@
>>>   obj-$(CONFIG_HAVE_CLK)        += clk-devres.o
>>>   obj-$(CONFIG_CLKDEV_LOOKUP)    += clkdev.o
>>>   obj-$(CONFIG_COMMON_CLK)    += clk.o clk-fixed-rate.o clk-gate.o \
>>> -                   clk-mux.o clk-divider.o clk-fixed-factor.o
>>> +                   clk-mux.o clk-divider.o clk-fixed-factor.o \
>>> +                   clk-composite.o
>> This list is getting a little out of hand. Should we sort it
>> alphabetically and put each file on one line?
>
> Do you want me to do it in this patch?
>
>

No.

>>> +static u8 clk_composite_get_parent(struct clk_hw *hw)
>>> +{
>>> +    struct clk_composite *composite = to_clk_composite(hw);
>>> +    const struct clk_ops *mux_ops = composite->mux_ops;
>>> +    struct clk_hw *mux_hw = composite->mux_hw;
>>> +
>>> +    mux_hw->clk = hw->clk;
>> Looks like this is already done down in the register function. Why are
>> we doing it again here and in each op?
>
> Some ops gets called during clk_init which is before clk_register
> returns.
>
>

Hmm. Ok.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 1/2] clk: Add composite clock type

[Mike Turquette]

Quoting Prashant Gaikwad (2013-01-03 21:51:45)
> Not all clocks are required to be decomposed into basic clock
> types but at the same time want to use the functionality
> provided by these basic clock types instead of duplicating.
> 
> For example, Tegra SoC has ~100 clocks which can be decomposed
> into Mux -> Div -> Gate clock types making the clock count to
> ~300. Also, parent change operation can not be performed on gate
> clock which forces to use mux clock in driver if want to change
> the parent.
> 
> Instead aggregate the basic clock types functionality into one
> clock and just use this clock for all operations. This clock
> type re-uses the functionality of basic clock types and not
> limited to basic clock types but any hardware-specific
> implementation.
> 
> Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com>

Hi Prashant,

I'm OK with the general concept of the composite clock, but a V2 patch
addressing all of the comments needs to be submitted first.  You might
base your patch on top of the following cleanup:


>From 135a829744067cb825b48f7422e22861e57d5ecd Mon Sep 17 00:00:00 2001
From: Mike Turquette <mturquette@linaro.org>
Date: Fri, 18 Jan 2013 13:00:05 -0800
Subject: [PATCH] clk: beautify Makefile

The list of common clock types was getting a bit unmanageable.  This
patch puts only one file on each line and reorders the object files
alphabetically.  Also a newline is added to separate the sections.

Reported-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Prashant Gaikwad <pgaikwad@nvidia.com>
Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
 drivers/clk/Makefile |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index ee90e87..e73b1d6 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -1,8 +1,13 @@
 # common clock types
 obj-$(CONFIG_HAVE_CLK)		+= clk-devres.o
 obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
-obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-fixed-rate.o clk-gate.o \
-				   clk-mux.o clk-divider.o clk-fixed-factor.o
+obj-$(CONFIG_COMMON_CLK)	+= clk.o
+obj-$(CONFIG_COMMON_CLK)	+= clk-divider.o
+obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-factor.o
+obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-rate.o
+obj-$(CONFIG_COMMON_CLK)	+= clk-gate.o
+obj-$(CONFIG_COMMON_CLK)	+= clk-mux.o
+
 # SoCs specific
 obj-$(CONFIG_ARCH_BCM2835)	+= clk-bcm2835.o
 obj-$(CONFIG_ARCH_NOMADIK)	+= clk-nomadik.o
-- 
1.7.10.4