[PATCH 0/4] Add framework to support clkout

[PATCH 0/4] Add framework to support clkout

[Tushar Behera]

The MUX/GATE register for XCLKOUT doesn't resides within PMU domain,
this can be accessed through a regmap provided by syscon driver. Adding
another clock provider to handle regmap based handing of XCLKOUT.

Dependency:
1. [PATCH v3] mfd: syscon: Support early initialization
http://article.gmane.org/gmane.linux.kernel/1679446

Also we need to find a suitable place to call early_syscon_init(), after
the device tree has been unflattened and before clock initialization.

While testing, I called this before of_clk_init() in arch/arm/kernel/time.c,
but that place is too generic. Calling anywhere from exynos.c is not
working ATM.

Tushar Behera (4):
  clk: samsung: out: Add infrastructure to register CLKOUT
  clk: samsung: exynos5420: Add xclkout debug clock
  clk: samsung: exynos5250: Add xclkout debug clock
  ARM: dts: Add pmu-syscon handle for Exynos5420/Exynos5250 clock

 arch/arm/boot/dts/exynos5250.dtsi      |    1 +
 arch/arm/boot/dts/exynos5420.dtsi      |    1 +
 drivers/clk/samsung/Makefile           |    2 +-
 drivers/clk/samsung/clk-exynos5250.c   |   14 +++
 drivers/clk/samsung/clk-exynos5420.c   |   14 +++
 drivers/clk/samsung/clk-out.c          |  181 ++++++++++++++++++++++++++++++++
 drivers/clk/samsung/clk.h              |   33 ++++++
 include/dt-bindings/clock/exynos5250.h |    3 +
 include/dt-bindings/clock/exynos5420.h |    5 +-
 9 files changed, 252 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clk/samsung/clk-out.c

-- 
1.7.9.5

[PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT

[Tushar Behera]

All SoC in Exynos-series have a clock with name XCLKOUT to provide
debug information about various clocks available in the SoC. The register
controlling the MUX and GATE of this clock is provided within PMU domain.
Since PMU domain can't be dedicatedly mapped by every driver, the register
needs to be handled through a regmap handle provided by PMU syscon
controller. Right now, CCF doesn't allow regmap based MUX and GATE clocks,
hence a dedicated clock provider for XCLKOUT is added here.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
CC: Tomasz Figa <t.figa@samsung.com>
---
 drivers/clk/samsung/Makefile  |    2 +-
 drivers/clk/samsung/clk-out.c |  181 +++++++++++++++++++++++++++++++++++++++++
 drivers/clk/samsung/clk.h     |   33 ++++++++
 3 files changed, 215 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/samsung/clk-out.c

diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index 8eb4799..d23ad4f 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -2,7 +2,7 @@
 # Samsung Clock specific Makefile
 #
 
-obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o
+obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o clk-out.o
 obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4.o
 obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
 obj-$(CONFIG_SOC_EXYNOS5420)	+= clk-exynos5420.o
diff --git a/drivers/clk/samsung/clk-out.c b/drivers/clk/samsung/clk-out.c
new file mode 100644
index 0000000..76489b6
--- /dev/null
+++ b/drivers/clk/samsung/clk-out.c
@@ -0,0 +1,181 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This file contains the utility functions to register the clkout clocks.
+*/
+
+/**
+ * All SoC in Exynos-series have a clock with name XCLKOUT to provide
+ * debug information about various clocks available in the SoC. The register
+ * controlling the MUX and GATE of this clock is provided within PMU domain.
+ * Since PMU domain can't be dedicatedly mapped every driver, the register
+ * needs to be handled through a regmap handle provided by PMU syscon
+ * controller. Right now, CCF doesn't allow regmap based MUX and GATE clocks,
+ * hence a dedicated clock provider for XCLKOUT is added here.
+ */
+
+#include <linux/errno.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+
+#include "clk.h"
+
+/**
+ * struct samsung_clkout_soc_data: SoC specific register details
+ * @reg: Offset of CLKOUT register from PMU base
+ * @mux_shift: Start-bit of MUX bit-field
+ * @mux_width: Width of MUX bit-field
+ * @enable_bit: The bit corresponding to gating of this clock
+ */
+struct samsung_clkout_soc_data {
+	unsigned int reg;
+	u8 mux_shift;
+	u8 mux_width;
+	u8 enable_bit;
+};
+
+/**
+ * struct samsung_clkout: Structure to store driver specific clock context
+ * @hw: Handle to CCF clock
+ * @soc_data: SoC specific register details
+ * @regmap: Regmap handle of the PMU
+ */
+struct samsung_clkout {
+	struct clk_hw hw;
+	const struct samsung_clkout_soc_data *soc_data;
+	struct regmap *regmap;
+};
+
+#define to_clk_out(_hw) container_of(_hw, struct samsung_clkout, hw)
+
+int samsung_clkout_enable(struct clk_hw *hw)
+{
+	struct samsung_clkout *clkout = to_clk_out(hw);
+	const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
+	unsigned int enable_mask = BIT(soc_data->enable_bit);
+
+	/* clkout is enabled if enable bit is low */
+	regmap_update_bits(clkout->regmap, soc_data->reg, enable_mask, 0);
+
+	return 0;
+}
+
+void samsung_clkout_disable(struct clk_hw *hw)
+{
+	struct samsung_clkout *clkout = to_clk_out(hw);
+	const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
+	unsigned int enable_mask = BIT(soc_data->enable_bit);
+
+	/* clkout is gated if enable bit is high */
+	regmap_update_bits(clkout->regmap, soc_data->reg,
+			enable_mask, enable_mask);
+
+	return;
+}
+
+int samsung_clkout_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct samsung_clkout *clkout = to_clk_out(hw);
+	const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
+	unsigned int parent_mask = BIT(soc_data->mux_width) - 1;
+
+	regmap_update_bits(clkout->regmap, soc_data->reg,
+			parent_mask << soc_data->mux_shift,
+			index << soc_data->mux_shift);
+
+	return 0;
+}
+
+u8 samsung_clkout_get_parent(struct clk_hw *hw)
+{
+	struct samsung_clkout *clkout = to_clk_out(hw);
+	const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
+	unsigned int parent_mask = BIT(soc_data->mux_width) - 1;
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(clkout->regmap, soc_data->reg, &val);
+
+	return (val >> soc_data->mux_shift) & parent_mask;
+}
+
+static const struct clk_ops samsung_clkout_clk_ops = {
+	.enable = samsung_clkout_enable,
+	.disable = samsung_clkout_disable,
+	.set_parent = samsung_clkout_set_parent,
+	.get_parent = samsung_clkout_get_parent,
+};
+
+static void __init _samsung_clk_register_clkout(
+		struct samsung_out_clock *out,
+		const struct samsung_clkout_soc_data *soc_data,
+		struct regmap *regmap)
+{
+	struct samsung_clkout *clkout;
+	struct clk *clk;
+	struct clk_init_data init;
+	int ret;
+
+	clkout = kzalloc(sizeof(*clkout), GFP_KERNEL);
+	if (!clkout) {
+		pr_err("%s: could not allocate out clk %s\n",
+			__func__, out->name);
+		return;
+	}
+
+	init.name = out->name;
+	init.parent_names = out->parent_names;
+	init.num_parents = out->num_parents;
+	init.ops = &samsung_clkout_clk_ops;
+
+	clkout->hw.init = &init;
+	clkout->regmap = regmap;
+	clkout->soc_data = soc_data;
+
+	clk = clk_register(NULL, &clkout->hw);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register out clock %s : %ld\n",
+			__func__, out->name, PTR_ERR(clk));
+		kfree(clkout);
+		return;
+	}
+
+	samsung_clk_add_lookup(clk, out->id);
+
+	if (!out->alias)
+		return;
+
+	ret = clk_register_clkdev(clk, out->alias, out->dev_name);
+	if (ret)
+		pr_err("%s: failed to register lookup for %s : %d",
+			__func__, out->name, ret);
+}
+
+/* All existing Exynos serial of SoCs have common values for this offsets. */
+static const struct samsung_clkout_soc_data exynos_clkout_soc_data = {
+	.reg = 0xa00,
+	.mux_shift = 8,
+	.mux_width = 5,
+	.enable_bit = 0,
+};
+
+void __init samsung_clk_register_clkout(struct device_node *np,
+		struct samsung_out_clock *out_clk_list, unsigned int nr_out_clk)
+{
+	int cnt;
+	struct regmap *reg;
+	const struct samsung_clkout_soc_data *priv = &exynos_clkout_soc_data;
+
+	reg = syscon_early_regmap_lookup_by_phandle(np, "samsung,pmu-syscon");
+	if (IS_ERR(reg)) {
+		pr_err("Failed to get pmu-syscon handle for clkout\n");
+		return;
+	}
+
+	for (cnt = 0; cnt < nr_out_clk; cnt++)
+		_samsung_clk_register_clkout(&out_clk_list[cnt], priv, reg);
+}
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index c7141ba..b4b2122 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -312,6 +312,37 @@ struct samsung_pll_clock {
 	__PLL(_typ, _id, NULL, _name, _pname, CLK_GET_RATE_NOCACHE,	\
 		_lock, _con, _rtable, _alias)
 
+/**
+ * struct samsung_out_clock: information about CLKOUT clock
+ * @id: platform specific id of the clock.
+ * @dev_name: name of the device to which this clock belongs.
+ * @name: name of this mux clock.
+ * @parent_names: array of pointer to parent clock names.
+ * @num_parents: number of parents listed in @parent_names.
+ * @alias: optional clock alias name to be assigned to this clock.
+ */
+struct samsung_out_clock {
+	unsigned int		id;
+	const char		*dev_name;
+	const char		*name;
+	const char		**parent_names;
+	unsigned int		num_parents;
+	const char		*alias;
+};
+
+#define __CLKOUT(_id, dname, cname, pnames, a)		\
+	{							\
+		.id		= _id,				\
+		.dev_name	= dname,			\
+		.name		= cname,			\
+		.parent_names	= pnames,			\
+		.num_parents	= ARRAY_SIZE(pnames),		\
+		.alias		= a,				\
+	}
+
+#define CLKOUT(_id, cname, pnames) \
+	__CLKOUT(_id, NULL, cname, pnames, NULL)
+
 extern void __init samsung_clk_init(struct device_node *np, void __iomem *base,
 				    unsigned long nr_clks);
 extern void __init samsung_clk_of_register_fixed_ext(
@@ -335,6 +366,8 @@ extern void __init samsung_clk_register_gate(
 		struct samsung_gate_clock *clk_list, unsigned int nr_clk);
 extern void __init samsung_clk_register_pll(struct samsung_pll_clock *pll_list,
 		unsigned int nr_clk, void __iomem *base);
+extern void __init samsung_clk_register_clkout(struct device_node *np,
+	struct samsung_out_clock *out_clk_list, unsigned int nr_out_clk);
 
 extern unsigned long _get_rate(const char *clk_name);
 
-- 
1.7.9.5

[PATCH 2/4] clk: samsung: exynos5420: Add xclkout debug clock

[Tushar Behera]

A new clock provider has been added to configure the XCLKOUT debug
clock. Added a minimal implemetation for Exynos5420 clock driver.

Right now, only one valid parent clock from XCLKOUT is defined
in existing clock driver. The driver will be updated later for other
for other parent clocks.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
CC: Tomasz Figa <t.figa@samsung.com>
---
 drivers/clk/samsung/clk-exynos5420.c   |   14 ++++++++++++++
 include/dt-bindings/clock/exynos5420.h |    5 ++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index 60b2681..a8f6527 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -296,6 +296,13 @@ PNAME(hdmi_p)	= { "dout_hdmi_pixel", "sclk_hdmiphy" };
 PNAME(maudio0_p)	= { "fin_pll", "maudio_clk", "sclk_dpll", "sclk_mpll",
 			  "sclk_spll", "sclk_ipll", "sclk_epll", "sclk_rpll" };
 
+PNAME(xclkout_p) = {
+	"dummy", "dummy", "dummy", "dummy",
+	"dummy", "dummy", "dummy", "dummy",
+	"dummy", "dummy", "dummy", "dummy",
+	"dummy", "dummy", "dummy", "dummy",
+	"fin_pll", "dummy", "dummy" };
+
 /* fixed rate clocks generated outside the soc */
 static struct samsung_fixed_rate_clock exynos5420_fixed_rate_ext_clks[] __initdata = {
 	FRATE(CLK_FIN_PLL, "fin_pll", NULL, CLK_IS_ROOT, 0),
@@ -308,6 +315,7 @@ static struct samsung_fixed_rate_clock exynos5420_fixed_rate_clks[] __initdata =
 	FRATE(0, "sclk_usbh20", NULL, CLK_IS_ROOT, 48000000),
 	FRATE(0, "mphy_refclk_ixtal24", NULL, CLK_IS_ROOT, 48000000),
 	FRATE(0, "sclk_usbh20_scan_clk", NULL, CLK_IS_ROOT, 480000000),
+	FRATE(0, "dummy", NULL, CLK_IS_ROOT, 0),
 };
 
 static struct samsung_fixed_factor_clock exynos5420_fixed_factor_clks[] __initdata = {
@@ -770,6 +778,10 @@ static struct samsung_pll_clock exynos5420_plls[nr_plls] __initdata = {
 		KPLL_CON0, NULL),
 };
 
+static struct samsung_out_clock exynos5420_clkout[] __initdata = {
+	CLKOUT(CLK_XCLKOUT, "xclkout", xclkout_p),
+};
+
 static struct of_device_id ext_clk_match[] __initdata = {
 	{ .compatible = "samsung,exynos5420-oscclk", .data = (void *)0, },
 	{ },
@@ -802,6 +814,8 @@ static void __init exynos5420_clk_init(struct device_node *np)
 			ARRAY_SIZE(exynos5420_div_clks));
 	samsung_clk_register_gate(exynos5420_gate_clks,
 			ARRAY_SIZE(exynos5420_gate_clks));
+	samsung_clk_register_clkout(np,
+			exynos5420_clkout, ARRAY_SIZE(exynos5420_clkout));
 
 	exynos5420_clk_sleep_init();
 }
diff --git a/include/dt-bindings/clock/exynos5420.h b/include/dt-bindings/clock/exynos5420.h
index 5eefd88..28eca32 100644
--- a/include/dt-bindings/clock/exynos5420.h
+++ b/include/dt-bindings/clock/exynos5420.h
@@ -182,7 +182,10 @@
 /* divider clocks */
 #define CLK_DOUT_PIXEL		768
 
+/* debug clocks */
+#define CLK_XCLKOUT		896
+
 /* must be greater than maximal clock id */
-#define CLK_NR_CLKS		769
+#define CLK_NR_CLKS		897
 
 #endif /* _DT_BINDINGS_CLOCK_EXYNOS_5420_H */
-- 
1.7.9.5

[PATCH 3/4] clk: samsung: exynos5250: Add xclkout debug clock

[Tushar Behera]

A new clock provider has been added to configure the XCLKOUT debug
clock. Added a minimal implemetation for Exynos5420 clock driver.

Right now, only one valid parent clock from XCLKOUT is defined
in existing clock driver. The driver will be updated later for other
for other parent clocks.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
CC: Tomasz Figa <t.figa@samsung.com>
---
 drivers/clk/samsung/clk-exynos5250.c   |   14 ++++++++++++++
 include/dt-bindings/clock/exynos5250.h |    3 +++
 2 files changed, 17 insertions(+)

diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index e7ee442..2637aea 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -225,6 +225,13 @@ PNAME(mout_audio2_p)	= { "cdclk2", "fin_pll", "sclk_hdmi27m", "sclk_dptxphy",
 PNAME(mout_spdif_p)	= { "sclk_audio0", "sclk_audio1", "sclk_audio2",
 				"spdif_extclk" };
 
+PNAME(xclkout_p) = {
+	"dummy", "dummy", "dummy", "dummy",
+	"dummy", "dummy", "dummy", "dummy",
+	"dummy", "dummy", "dummy", "dummy",
+	"dummy", "dummy", "dummy", "dummy",
+	"fin_pll", "dummy", "dummy" };
+
 /* fixed rate clocks generated outside the soc */
 static struct samsung_fixed_rate_clock exynos5250_fixed_rate_ext_clks[] __initdata = {
 	FRATE(CLK_FIN_PLL, "fin_pll", NULL, CLK_IS_ROOT, 0),
@@ -236,6 +243,7 @@ static struct samsung_fixed_rate_clock exynos5250_fixed_rate_clks[] __initdata =
 	FRATE(0, "sclk_hdmi27m", NULL, CLK_IS_ROOT, 27000000),
 	FRATE(0, "sclk_dptxphy", NULL, CLK_IS_ROOT, 24000000),
 	FRATE(0, "sclk_uhostphy", NULL, CLK_IS_ROOT, 48000000),
+	FRATE(0, "dummy", NULL, CLK_IS_ROOT, 0),
 };
 
 static struct samsung_fixed_factor_clock exynos5250_fixed_factor_clks[] __initdata = {
@@ -678,6 +686,10 @@ static struct samsung_pll_clock exynos5250_plls[nr_plls] __initdata = {
 		VPLL_LOCK, VPLL_CON0, NULL),
 };
 
+static struct samsung_out_clock exynos5250_clkout[] __initdata = {
+	CLKOUT(CLK_XCLKOUT, "xclkout", xclkout_p),
+};
+
 static struct of_device_id ext_clk_match[] __initdata = {
 	{ .compatible = "samsung,clock-xxti", .data = (void *)0, },
 	{ },
@@ -721,6 +733,8 @@ static void __init exynos5250_clk_init(struct device_node *np)
 			ARRAY_SIZE(exynos5250_div_clks));
 	samsung_clk_register_gate(exynos5250_gate_clks,
 			ARRAY_SIZE(exynos5250_gate_clks));
+	samsung_clk_register_clkout(np,
+			exynos5250_clkout, ARRAY_SIZE(exynos5250_clkout));
 
 	exynos5250_clk_sleep_init();
 
diff --git a/include/dt-bindings/clock/exynos5250.h b/include/dt-bindings/clock/exynos5250.h
index 922f2dc..7c1cd0b 100644
--- a/include/dt-bindings/clock/exynos5250.h
+++ b/include/dt-bindings/clock/exynos5250.h
@@ -151,6 +151,9 @@
 #define CLK_MDMA0		346
 #define CLK_SMMU_MDMA0		347
 
+/* debug clocks */
+#define CLK_XCLKOUT		896
+
 /* mux clocks */
 #define CLK_MOUT_HDMI		1024
 
-- 
1.7.9.5

[PATCH 4/4] ARM: dts: Add pmu-syscon handle for Exynos5420/Exynos5250 clock

[Tushar Behera]

XCLKOUT in Exynos5420/Exynos5250 is controlled through a register in
PMU domain. Pass pmu-syscon handle to the clock driver so that XCLKOUT
can be properly configured.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
CC: Kukjin Kim <kgene.kim@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi |    1 +
 arch/arm/boot/dts/exynos5420.dtsi |    1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 3742331..214db94 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -86,6 +86,7 @@
 		compatible = "samsung,exynos5250-clock";
 		reg = <0x10010000 0x30000>;
 		#clock-cells = <1>;
+		samsung,pmu-syscon = <&pmu_system_controller>;
 	};
 
 	clock_audss: audss-clock-controller@3810000 {
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index c3a9a66..489bd08 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -114,6 +114,7 @@
 		compatible = "samsung,exynos5420-clock";
 		reg = <0x10010000 0x30000>;
 		#clock-cells = <1>;
+		samsung,pmu-syscon = <&pmu_system_controller>;
 	};
 
 	clock_audss: audss-clock-controller@3810000 {
-- 
1.7.9.5

Re: [PATCH 0/4] Add framework to support clkout

[Pankaj Dubey]

Hi Tushar,

On 05/09/2014 10:00 PM, Tushar Behera wrote:
> The MUX/GATE register for XCLKOUT doesn't resides within PMU domain,
> this can be accessed through a regmap provided by syscon driver. Adding
> another clock provider to handle regmap based handing of XCLKOUT.
>
> Dependency:
> 1. [PATCH v3] mfd: syscon: Support early initialization
> http://article.gmane.org/gmane.linux.kernel/1679446
>
> Also we need to find a suitable place to call early_syscon_init(), after
> the device tree has been unflattened and before clock initialization.
>
> While testing, I called this before of_clk_init() in arch/arm/kernel/time.c,
> but that place is too generic. Calling anywhere from exynos.c is not
> working ATM.

IMO we do not need to, or if I am not wrong we should not change time.c.

It's possible if we have exynos specific init_time with following changes.
FYI, In my patch series for Exynos PMU [1], currently I am handling this in
exynos_dt_machine_init. But definitely it can be handled as below and it works
without any side effect and I have tested it. Only reason I do not adopted this
as for Exynos PMU patch support I had other options. But if required and if
following change is acceptable I can include this in my next version of Exynos
PMU patch series.

[1]: https://lkml.org/lkml/2014/4/30/18


+static void __init exynos_init_time(void)
+{
+    /* Nothing to do timer specific
+     * as early_syscon_init requires DT to be unflattened and
+     * system should be able to allocate memory we need to
+     * postpone until init_time, but it should be done before
+     * init_machine. Because before init_machine, secondary
+     * core boot starts and it uses PMU registers.
+     */
+
+    exynos_map_pmu();
+
+    of_clk_init(NULL);
+    clocksource_of_init();
+
+}
+
  DT_MACHINE_START(EXYNOS_DT, "SAMSUNG EXYNOS (Flattened Device Tree)")
      /* Maintainer: Thomas Abraham <thomas.abraham@linaro.org> */
      /* Maintainer: Kukjin Kim <kgene.kim@samsung.com> */
      .smp        = smp_ops(exynos_smp_ops),
      .map_io        = exynos_init_io,
      .init_early    = exynos_firmware_init,
+    .init_time    = exynos_init_time,

>
> Tushar Behera (4):
>    clk: samsung: out: Add infrastructure to register CLKOUT
>    clk: samsung: exynos5420: Add xclkout debug clock
>    clk: samsung: exynos5250: Add xclkout debug clock
>    ARM: dts: Add pmu-syscon handle for Exynos5420/Exynos5250 clock
>
>   arch/arm/boot/dts/exynos5250.dtsi      |    1 +
>   arch/arm/boot/dts/exynos5420.dtsi      |    1 +
>   drivers/clk/samsung/Makefile           |    2 +-
>   drivers/clk/samsung/clk-exynos5250.c   |   14 +++
>   drivers/clk/samsung/clk-exynos5420.c   |   14 +++
>   drivers/clk/samsung/clk-out.c          |  181 ++++++++++++++++++++++++++++++++
>   drivers/clk/samsung/clk.h              |   33 ++++++
>   include/dt-bindings/clock/exynos5250.h |    3 +
>   include/dt-bindings/clock/exynos5420.h |    5 +-
>   9 files changed, 252 insertions(+), 2 deletions(-)
>   create mode 100644 drivers/clk/samsung/clk-out.c
>


-- 
Best Regards,
Pankaj Dubey

Re: [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT

[Pankaj Dubey]

On 05/09/2014 10:00 PM, Tushar Behera wrote:
> All SoC in Exynos-series have a clock with name XCLKOUT to provide
> debug information about various clocks available in the SoC. The register
> controlling the MUX and GATE of this clock is provided within PMU domain.
> Since PMU domain can't be dedicatedly mapped by every driver, the register
> needs to be handled through a regmap handle provided by PMU syscon
> controller. Right now, CCF doesn't allow regmap based MUX and GATE clocks,
> hence a dedicated clock provider for XCLKOUT is added here.
>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> CC: Tomasz Figa <t.figa@samsung.com>
> ---
>   drivers/clk/samsung/Makefile  |    2 +-
>   drivers/clk/samsung/clk-out.c |  181 +++++++++++++++++++++++++++++++++++++++++
>   drivers/clk/samsung/clk.h     |   33 ++++++++
>   3 files changed, 215 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/clk/samsung/clk-out.c
>
> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
> index 8eb4799..d23ad4f 100644
> --- a/drivers/clk/samsung/Makefile
> +++ b/drivers/clk/samsung/Makefile
> @@ -2,7 +2,7 @@
>   # Samsung Clock specific Makefile
>   #
>   
> -obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o
> +obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o clk-out.o
>   obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4.o
>   obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
>   obj-$(CONFIG_SOC_EXYNOS5420)	+= clk-exynos5420.o
> diff --git a/drivers/clk/samsung/clk-out.c b/drivers/clk/samsung/clk-out.c
> new file mode 100644
> index 0000000..76489b6
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-out.c
> @@ -0,0 +1,181 @@
> +/*
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This file contains the utility functions to register the clkout clocks.
> +*/
> +
> +/**
> + * All SoC in Exynos-series have a clock with name XCLKOUT to provide
> + * debug information about various clocks available in the SoC. The register
> + * controlling the MUX and GATE of this clock is provided within PMU domain.
> + * Since PMU domain can't be dedicatedly mapped every driver, the register
> + * needs to be handled through a regmap handle provided by PMU syscon
> + * controller. Right now, CCF doesn't allow regmap based MUX and GATE clocks,
> + * hence a dedicated clock provider for XCLKOUT is added here.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +
> +#include "clk.h"
> +
> +/**
> + * struct samsung_clkout_soc_data: SoC specific register details
> + * @reg: Offset of CLKOUT register from PMU base

how about naming this variable as "offset" instead of "reg".

> + * @mux_shift: Start-bit of MUX bit-field
> + * @mux_width: Width of MUX bit-field
> + * @enable_bit: The bit corresponding to gating of this clock
> + */
> +struct samsung_clkout_soc_data {
> +	unsigned int reg;
> +	u8 mux_shift;
> +	u8 mux_width;
> +	u8 enable_bit;
> +};
> +
> +/**
> + * struct samsung_clkout: Structure to store driver specific clock context
> + * @hw: Handle to CCF clock
> + * @soc_data: SoC specific register details
> + * @regmap: Regmap handle of the PMU
> + */
> +struct samsung_clkout {
> +	struct clk_hw hw;
> +	const struct samsung_clkout_soc_data *soc_data;
> +	struct regmap *regmap;
> +};
> +
> +#define to_clk_out(_hw) container_of(_hw, struct samsung_clkout, hw)
> +
> +int samsung_clkout_enable(struct clk_hw *hw)
> +{
> +	struct samsung_clkout *clkout = to_clk_out(hw);
> +	const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
> +	unsigned int enable_mask = BIT(soc_data->enable_bit);
> +
> +	/* clkout is enabled if enable bit is low */
> +	regmap_update_bits(clkout->regmap, soc_data->reg, enable_mask, 0);
> +
> +	return 0;
> +}
> +
> +void samsung_clkout_disable(struct clk_hw *hw)
> +{
> +	struct samsung_clkout *clkout = to_clk_out(hw);
> +	const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
> +	unsigned int enable_mask = BIT(soc_data->enable_bit);
> +
> +	/* clkout is gated if enable bit is high */
> +	regmap_update_bits(clkout->regmap, soc_data->reg,
> +			enable_mask, enable_mask);
> +
> +	return;
> +}
> +
> +int samsung_clkout_set_parent(struct clk_hw *hw, u8 index)
> +{
> +	struct samsung_clkout *clkout = to_clk_out(hw);
> +	const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
> +	unsigned int parent_mask = BIT(soc_data->mux_width) - 1;
> +
> +	regmap_update_bits(clkout->regmap, soc_data->reg,
> +			parent_mask << soc_data->mux_shift,
> +			index << soc_data->mux_shift);
> +
> +	return 0;
> +}
> +
> +u8 samsung_clkout_get_parent(struct clk_hw *hw)
> +{
> +	struct samsung_clkout *clkout = to_clk_out(hw);
> +	const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
> +	unsigned int parent_mask = BIT(soc_data->mux_width) - 1;
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(clkout->regmap, soc_data->reg, &val);

Do we really need to keep return value in "ret" as I can't see you are 
using it anywhere?

> +
> +	return (val >> soc_data->mux_shift) & parent_mask;
> +}
> +
> +static const struct clk_ops samsung_clkout_clk_ops = {
> +	.enable = samsung_clkout_enable,
> +	.disable = samsung_clkout_disable,
> +	.set_parent = samsung_clkout_set_parent,
> +	.get_parent = samsung_clkout_get_parent,
> +};
> +
> +static void __init _samsung_clk_register_clkout(
> +		struct samsung_out_clock *out,
> +		const struct samsung_clkout_soc_data *soc_data,
> +		struct regmap *regmap)
> +{
> +	struct samsung_clkout *clkout;
> +	struct clk *clk;
> +	struct clk_init_data init;
> +	int ret;
> +
> +	clkout = kzalloc(sizeof(*clkout), GFP_KERNEL);
> +	if (!clkout) {
> +		pr_err("%s: could not allocate out clk %s\n",
> +			__func__, out->name);
> +		return;
> +	}
> +
> +	init.name = out->name;
> +	init.parent_names = out->parent_names;
> +	init.num_parents = out->num_parents;
> +	init.ops = &samsung_clkout_clk_ops;
> +
> +	clkout->hw.init = &init;
> +	clkout->regmap = regmap;
> +	clkout->soc_data = soc_data;
> +
> +	clk = clk_register(NULL, &clkout->hw);
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: failed to register out clock %s : %ld\n",
> +			__func__, out->name, PTR_ERR(clk));
> +		kfree(clkout);
> +		return;
> +	}
> +
> +	samsung_clk_add_lookup(clk, out->id);
> +
> +	if (!out->alias)
> +		return;
> +
> +	ret = clk_register_clkdev(clk, out->alias, out->dev_name);
> +	if (ret)
> +		pr_err("%s: failed to register lookup for %s : %d",
> +			__func__, out->name, ret);
> +}
> +
> +/* All existing Exynos serial of SoCs have common values for this offsets. */
typo: serial/series/
> +static const struct samsung_clkout_soc_data exynos_clkout_soc_data = {
> +	.reg = 0xa00,
> +	.mux_shift = 8,
> +	.mux_width = 5,
> +	.enable_bit = 0,
> +};
> +
> +void __init samsung_clk_register_clkout(struct device_node *np,
> +		struct samsung_out_clock *out_clk_list, unsigned int nr_out_clk)
> +{
> +	int cnt;
> +	struct regmap *reg;
> +	const struct samsung_clkout_soc_data *priv = &exynos_clkout_soc_data;
> +
> +	reg = syscon_early_regmap_lookup_by_phandle(np, "samsung,pmu-syscon");
> +	if (IS_ERR(reg)) {
> +		pr_err("Failed to get pmu-syscon handle for clkout\n");
> +		return;
> +	}
> +
> +	for (cnt = 0; cnt < nr_out_clk; cnt++)
> +		_samsung_clk_register_clkout(&out_clk_list[cnt], priv, reg);
> +}
> diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
> index c7141ba..b4b2122 100644
> --- a/drivers/clk/samsung/clk.h
> +++ b/drivers/clk/samsung/clk.h
> @@ -312,6 +312,37 @@ struct samsung_pll_clock {
>   	__PLL(_typ, _id, NULL, _name, _pname, CLK_GET_RATE_NOCACHE,	\
>   		_lock, _con, _rtable, _alias)
>   
> +/**
> + * struct samsung_out_clock: information about CLKOUT clock
> + * @id: platform specific id of the clock.
> + * @dev_name: name of the device to which this clock belongs.
> + * @name: name of this mux clock.
> + * @parent_names: array of pointer to parent clock names.
> + * @num_parents: number of parents listed in @parent_names.
> + * @alias: optional clock alias name to be assigned to this clock.
> + */
> +struct samsung_out_clock {
> +	unsigned int		id;
> +	const char		*dev_name;
> +	const char		*name;
> +	const char		**parent_names;
> +	unsigned int		num_parents;
> +	const char		*alias;
> +};
> +
> +#define __CLKOUT(_id, dname, cname, pnames, a)		\
> +	{							\
> +		.id		= _id,				\
> +		.dev_name	= dname,			\
> +		.name		= cname,			\
> +		.parent_names	= pnames,			\
> +		.num_parents	= ARRAY_SIZE(pnames),		\
> +		.alias		= a,				\
> +	}
> +
> +#define CLKOUT(_id, cname, pnames) \
> +	__CLKOUT(_id, NULL, cname, pnames, NULL)
> +
>   extern void __init samsung_clk_init(struct device_node *np, void __iomem *base,
>   				    unsigned long nr_clks);
>   extern void __init samsung_clk_of_register_fixed_ext(
> @@ -335,6 +366,8 @@ extern void __init samsung_clk_register_gate(
>   		struct samsung_gate_clock *clk_list, unsigned int nr_clk);
>   extern void __init samsung_clk_register_pll(struct samsung_pll_clock *pll_list,
>   		unsigned int nr_clk, void __iomem *base);
> +extern void __init samsung_clk_register_clkout(struct device_node *np,
> +	struct samsung_out_clock *out_clk_list, unsigned int nr_out_clk);
>   
>   extern unsigned long _get_rate(const char *clk_name);
>   


-- 
Best Regards,
Pankaj Dubey

Re: [PATCH 0/4] Add framework to support clkout

[Tushar Behera]

On 05/10/2014 09:09 AM, Pankaj Dubey wrote:
> Hi Tushar,
> 
[ ... ]
>> Also we need to find a suitable place to call early_syscon_init(), after
>> the device tree has been unflattened and before clock initialization.
>>
>> While testing, I called this before of_clk_init() in
>> arch/arm/kernel/time.c,
>> but that place is too generic. Calling anywhere from exynos.c is not
>> working ATM.
> 
> IMO we do not need to, or if I am not wrong we should not change time.c.
> 

The above solution is definitely a hack and just to test my stuff. The
below solution looks good.

> It's possible if we have exynos specific init_time with following changes.
> FYI, In my patch series for Exynos PMU [1], currently I am handling this in
> exynos_dt_machine_init. But definitely it can be handled as below and it
> works
> without any side effect and I have tested it. Only reason I do not
> adopted this
> as for Exynos PMU patch support I had other options. But if required and if
> following change is acceptable I can include this in my next version of
> Exynos
> PMU patch series.
> 
> [1]: https://lkml.org/lkml/2014/4/30/18
> 
> 
> +static void __init exynos_init_time(void)
> +{
> +    /* Nothing to do timer specific
> +     * as early_syscon_init requires DT to be unflattened and
> +     * system should be able to allocate memory we need to
> +     * postpone until init_time, but it should be done before
> +     * init_machine. Because before init_machine, secondary
> +     * core boot starts and it uses PMU registers.
> +     */
> +
> +    exynos_map_pmu();
> +

Instead of calling early_syscon_init() from within exynos_map_pmu(), it
would be good to call it explicitly here before exynos_map_pmu().

> +    of_clk_init(NULL);
> +    clocksource_of_init();
> +
> +}
> +

-- 
Tushar Behera

Re: [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT

[Tushar Behera]

On 05/10/2014 09:21 AM, Pankaj Dubey wrote:
> On 05/09/2014 10:00 PM, Tushar Behera wrote:
>> All SoC in Exynos-series have a clock with name XCLKOUT to provide
>> debug information about various clocks available in the SoC. The register
>> controlling the MUX and GATE of this clock is provided within PMU domain.
>> Since PMU domain can't be dedicatedly mapped by every driver, the
>> register
>> needs to be handled through a regmap handle provided by PMU syscon
>> controller. Right now, CCF doesn't allow regmap based MUX and GATE
>> clocks,
>> hence a dedicated clock provider for XCLKOUT is added here.
>>
>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>> CC: Tomasz Figa <t.figa@samsung.com>
>> ---
>>   drivers/clk/samsung/Makefile  |    2 +-
>>   drivers/clk/samsung/clk-out.c |  181
>> +++++++++++++++++++++++++++++++++++++++++
>>   drivers/clk/samsung/clk.h     |   33 ++++++++
>>   3 files changed, 215 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/clk/samsung/clk-out.c
>>

[ ... ]

>> +/**
>> + * struct samsung_clkout_soc_data: SoC specific register details
>> + * @reg: Offset of CLKOUT register from PMU base
> 
> how about naming this variable as "offset" instead of "reg".
> 

Okay, I will change that.

[ ... ]

>> +u8 samsung_clkout_get_parent(struct clk_hw *hw)
>> +{
>> +    struct samsung_clkout *clkout = to_clk_out(hw);
>> +    const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
>> +    unsigned int parent_mask = BIT(soc_data->mux_width) - 1;
>> +    unsigned int val;
>> +    int ret;
>> +
>> +    ret = regmap_read(clkout->regmap, soc_data->reg, &val);
> 
> Do we really need to keep return value in "ret" as I can't see you are
> using it anywhere?
> 

Right, we are not using that and can be removed.

>> +
>> +    return (val >> soc_data->mux_shift) & parent_mask;
>> +}
>> +

[ ... ]

>> +/* All existing Exynos serial of SoCs have common values for this
>> offsets. */
> typo: serial/series/

Sure. Thanks for your review.

-- 
Tushar Behera

Re: [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT

[Rahul Sharma]

Hi Tushar,

Basically you are adding a new clock-type for Clkout. IMO clkout
is not a special hardware. Existing clock types can be reused to
support clkout. I see 3 major problem here:

1) Clkout -> (Mux + Gate). You clubbed mux and gate together, and
exposing as a single clock which is something like a composite clock.
IMO this is not a recommended way in CCF.

2) New Clock Type: Since clkout is just a combination of a simple
mux and gate which are already supported, it is a unnecessary
duplication.

3) Clkout registered along with CMU: which is not correct. Clkout is in PMU
(Separate physical IP) and should be registered as a independent Clock
provider which provides 1 mux and 1 gate clock (As if now). It should also be
well connected with main CMU.

I understand the challenge in using regmap interface for a clock provider. But
we need to identify a clean solution. IMHO a independent clock provider with
iomap, is relatively cleaner approach till CCF is not ready with regmap based
reg access for clock registers.

Experts!! please comment.

Regards,
Rahul Sharma.

On 12 May 2014 10:16, Tushar Behera <tushar.behera@linaro.org> wrote:
> On 05/10/2014 09:21 AM, Pankaj Dubey wrote:
>> On 05/09/2014 10:00 PM, Tushar Behera wrote:
>>> All SoC in Exynos-series have a clock with name XCLKOUT to provide
>>> debug information about various clocks available in the SoC. The register
>>> controlling the MUX and GATE of this clock is provided within PMU domain.
>>> Since PMU domain can't be dedicatedly mapped by every driver, the
>>> register
>>> needs to be handled through a regmap handle provided by PMU syscon
>>> controller. Right now, CCF doesn't allow regmap based MUX and GATE
>>> clocks,
>>> hence a dedicated clock provider for XCLKOUT is added here.
>>>
>>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>> CC: Tomasz Figa <t.figa@samsung.com>
>>> ---
>>>   drivers/clk/samsung/Makefile  |    2 +-
>>>   drivers/clk/samsung/clk-out.c |  181
>>> +++++++++++++++++++++++++++++++++++++++++
>>>   drivers/clk/samsung/clk.h     |   33 ++++++++
>>>   3 files changed, 215 insertions(+), 1 deletion(-)
>>>   create mode 100644 drivers/clk/samsung/clk-out.c
>>>
>
> [ ... ]
>
>>> +/**
>>> + * struct samsung_clkout_soc_data: SoC specific register details
>>> + * @reg: Offset of CLKOUT register from PMU base
>>
>> how about naming this variable as "offset" instead of "reg".
>>
>
> Okay, I will change that.
>
> [ ... ]
>
>>> +u8 samsung_clkout_get_parent(struct clk_hw *hw)
>>> +{
>>> +    struct samsung_clkout *clkout = to_clk_out(hw);
>>> +    const struct samsung_clkout_soc_data *soc_data = clkout->soc_data;
>>> +    unsigned int parent_mask = BIT(soc_data->mux_width) - 1;
>>> +    unsigned int val;
>>> +    int ret;
>>> +
>>> +    ret = regmap_read(clkout->regmap, soc_data->reg, &val);
>>
>> Do we really need to keep return value in "ret" as I can't see you are
>> using it anywhere?
>>
>
> Right, we are not using that and can be removed.
>
>>> +
>>> +    return (val >> soc_data->mux_shift) & parent_mask;
>>> +}
>>> +
>
> [ ... ]
>
>>> +/* All existing Exynos serial of SoCs have common values for this
>>> offsets. */
>> typo: serial/series/
>
> Sure. Thanks for your review.
>
> --
> Tushar Behera
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT

[Tomasz Figa]

Hi Rahul, Tushar,

On 15.05.2014 15:44, Rahul Sharma wrote:
> Hi Tushar,
> 
> Basically you are adding a new clock-type for Clkout. IMO clkout
> is not a special hardware. Existing clock types can be reused to
> support clkout. I see 3 major problem here:
> 
> 1) Clkout -> (Mux + Gate). You clubbed mux and gate together, and
> exposing as a single clock which is something like a composite clock.
> IMO this is not a recommended way in CCF.
> 
> 2) New Clock Type: Since clkout is just a combination of a simple
> mux and gate which are already supported, it is a unnecessary
> duplication.
> 
> 3) Clkout registered along with CMU: which is not correct. Clkout is in PMU
> (Separate physical IP) and should be registered as a independent Clock
> provider which provides 1 mux and 1 gate clock (As if now). It should also be
> well connected with main CMU.
> 
> I understand the challenge in using regmap interface for a clock provider. But
> we need to identify a clean solution. IMHO a independent clock provider with
> iomap, is relatively cleaner approach till CCF is not ready with regmap based
> reg access for clock registers.
> 
> Experts!! please comment.

It's quite unfortunate that Tushar has duplicated the effort to create a
clkout driver, considering the fact that we did have such driver
internally at SRPOL and it was quite nice and simple.

I will post a cleaned-up version today, that is about 2 times smaller in
terms of lines of added code and provides the same functionality,
without introducing custom clock types. In addition, it models the
clkout properly as a feature of PMU, not CMU (CMU only provides outputs
of particular sub-blocks that are fed into the PMU).

Best regards,
Tomasz

Re: [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT

[Rahul Sharma]

On 15 May 2014 19:37, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Rahul, Tushar,
>
> On 15.05.2014 15:44, Rahul Sharma wrote:
>> Hi Tushar,
>>
>> Basically you are adding a new clock-type for Clkout. IMO clkout
>> is not a special hardware. Existing clock types can be reused to
>> support clkout. I see 3 major problem here:
>>
>> 1) Clkout -> (Mux + Gate). You clubbed mux and gate together, and
>> exposing as a single clock which is something like a composite clock.
>> IMO this is not a recommended way in CCF.
>>
>> 2) New Clock Type: Since clkout is just a combination of a simple
>> mux and gate which are already supported, it is a unnecessary
>> duplication.
>>
>> 3) Clkout registered along with CMU: which is not correct. Clkout is in PMU
>> (Separate physical IP) and should be registered as a independent Clock
>> provider which provides 1 mux and 1 gate clock (As if now). It should also be
>> well connected with main CMU.
>>
>> I understand the challenge in using regmap interface for a clock provider. But
>> we need to identify a clean solution. IMHO a independent clock provider with
>> iomap, is relatively cleaner approach till CCF is not ready with regmap based
>> reg access for clock registers.
>>
>> Experts!! please comment.
>
> It's quite unfortunate that Tushar has duplicated the effort to create a
> clkout driver, considering the fact that we did have such driver
> internally at SRPOL and it was quite nice and simple.
>
> I will post a cleaned-up version today, that is about 2 times smaller in
> terms of lines of added code and provides the same functionality,
> without introducing custom clock types. In addition, it models the
> clkout properly as a feature of PMU, not CMU (CMU only provides outputs
> of particular sub-blocks that are fed into the PMU).

oh.. thats nice. Please share the implementation.

Regards,
Rahul Sharma.

>
> Best regards,
> Tomasz
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT

[Tushar Behera]

On 15 May 2014 19:37, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Rahul, Tushar,
>
> On 15.05.2014 15:44, Rahul Sharma wrote:
>> Hi Tushar,
>>
>> Basically you are adding a new clock-type for Clkout. IMO clkout
>> is not a special hardware. Existing clock types can be reused to
>> support clkout. I see 3 major problem here:
>>
>> 1) Clkout -> (Mux + Gate). You clubbed mux and gate together, and
>> exposing as a single clock which is something like a composite clock.
>> IMO this is not a recommended way in CCF.
>>
>> 2) New Clock Type: Since clkout is just a combination of a simple
>> mux and gate which are already supported, it is a unnecessary
>> duplication.
>>
>> 3) Clkout registered along with CMU: which is not correct. Clkout is in PMU
>> (Separate physical IP) and should be registered as a independent Clock
>> provider which provides 1 mux and 1 gate clock (As if now). It should also be
>> well connected with main CMU.
>>
>> I understand the challenge in using regmap interface for a clock provider. But
>> we need to identify a clean solution. IMHO a independent clock provider with
>> iomap, is relatively cleaner approach till CCF is not ready with regmap based
>> reg access for clock registers.
>>
>> Experts!! please comment.
>
> It's quite unfortunate that Tushar has duplicated the effort to create a
> clkout driver, considering the fact that we did have such driver
> internally at SRPOL and it was quite nice and simple.
>

I had no idea that you had some solutions to this available to be posted :(

Now that the new series is posted, I will test that at my end and
update you later.

> I will post a cleaned-up version today, that is about 2 times smaller in
> terms of lines of added code and provides the same functionality,
> without introducing custom clock types. In addition, it models the
> clkout properly as a feature of PMU, not CMU (CMU only provides outputs
> of particular sub-blocks that are fed into the PMU).
>
> Best regards,
> Tomasz



-- 
Tushar Behera

Re: [PATCH 1/4] clk: samsung: out: Add infrastructure to register CLKOUT

[Tomasz Figa]

On 19.05.2014 05:30, Tushar Behera wrote:
> On 15 May 2014 19:37, Tomasz Figa <t.figa@samsung.com> wrote:
>> Hi Rahul, Tushar,
>>
>> On 15.05.2014 15:44, Rahul Sharma wrote:
>>> Hi Tushar,
>>>
>>> Basically you are adding a new clock-type for Clkout. IMO clkout
>>> is not a special hardware. Existing clock types can be reused to
>>> support clkout. I see 3 major problem here:
>>>
>>> 1) Clkout -> (Mux + Gate). You clubbed mux and gate together, and
>>> exposing as a single clock which is something like a composite clock.
>>> IMO this is not a recommended way in CCF.
>>>
>>> 2) New Clock Type: Since clkout is just a combination of a simple
>>> mux and gate which are already supported, it is a unnecessary
>>> duplication.
>>>
>>> 3) Clkout registered along with CMU: which is not correct. Clkout is in PMU
>>> (Separate physical IP) and should be registered as a independent Clock
>>> provider which provides 1 mux and 1 gate clock (As if now). It should also be
>>> well connected with main CMU.
>>>
>>> I understand the challenge in using regmap interface for a clock provider. But
>>> we need to identify a clean solution. IMHO a independent clock provider with
>>> iomap, is relatively cleaner approach till CCF is not ready with regmap based
>>> reg access for clock registers.
>>>
>>> Experts!! please comment.
>>
>> It's quite unfortunate that Tushar has duplicated the effort to create a
>> clkout driver, considering the fact that we did have such driver
>> internally at SRPOL and it was quite nice and simple.
>>
> 
> I had no idea that you had some solutions to this available to be posted :(
> 
> Now that the new series is posted, I will test that at my end and
> update you later.

Again, sorry for this duplicated effort.

The biggest problem is that we (SRPOL and Linaro) don't have a way to
share our upstream TODO lists and say "hey, we have this working, if you
need it, we can upstream it \o/" or "we need it too, but don't have
resources right now to work on it :(".

Maybe we should set up some wiki or any other semi-public site for this
purpose? I need to talk with other people from my team and my manager to
see what we can do. Other guys (other Samsung teams, Chromium folks,
hobbyists) could also benefit from it.

Anyway, thanks for your understanding and testing.

Best regards,
Tomasz