linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework
@ 2014-07-11  9:37 Tushar Behera
  2014-07-11  9:37 ` [PATCH 1/2] clk: exynos-audss: Simplify code to get clock names Tushar Behera
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Tushar Behera @ 2014-07-11  9:37 UTC (permalink / raw)
  To: linux-arm-kernel

The patchset is targetted as moving exising exynos-audss clock driver from being
a module driver. The driver is now registered through CLK_OF_DECLARE and is
inline with other Samsung clock drivers.

The patches are tested on Exynos5800 based Peach-Pi board. More tests are
welcome.

Tushar Behera (2):
  clk: exynos-audss: Simplify code to get clock names
  clk: exynos-audss: Update as per existing framework

 drivers/clk/samsung/clk-exynos-audss.c |  268 ++++++++++++--------------------
 1 file changed, 102 insertions(+), 166 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] clk: exynos-audss: Simplify code to get clock names
  2014-07-11  9:37 [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework Tushar Behera
@ 2014-07-11  9:37 ` Tushar Behera
  2014-07-11  9:37 ` [PATCH 2/2] clk: exynos-audss: Update as per existing framework Tushar Behera
  2014-07-11  9:51 ` [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework Tomasz Figa
  2 siblings, 0 replies; 7+ messages in thread
From: Tushar Behera @ 2014-07-11  9:37 UTC (permalink / raw)
  To: linux-arm-kernel

Instead of getting the clock names individually, it would be good to put
the logic within a loop.

Signed-off-by: Tushar Behera <tushar.b@samsung.com>
---
 drivers/clk/samsung/clk-exynos-audss.c |   33 ++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
index 13eae14c..ebfc5da 100644
--- a/drivers/clk/samsung/clk-exynos-audss.c
+++ b/drivers/clk/samsung/clk-exynos-audss.c
@@ -80,10 +80,14 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
 	int i, ret = 0;
 	struct resource *res;
+	struct clk *tmp;
+	const char *clk_name_ref[] = {
+		"pll_ref", "pll_in", "cdclk", "sclk_audio", "sclk_pcm_in" };
+	const char *clk_name_actual[] = {
+		"fin_pll", "fout_epll", "cdclk0", "sclk_audio0", "sclk_pcm0"};
 	const char *mout_audss_p[] = {"fin_pll", "fout_epll"};
 	const char *mout_i2s_p[] = {"mout_audss", "cdclk0", "sclk_audio0"};
 	const char *sclk_pcm_p = "sclk_pcm0";
-	struct clk *pll_ref, *pll_in, *cdclk, *sclk_audio, *sclk_pcm_in;
 	const struct of_device_id *match;
 	enum exynos_audss_clk_type variant;
 
@@ -111,23 +115,23 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	else
 		clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS - 1;
 
-	pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
-	pll_in = devm_clk_get(&pdev->dev, "pll_in");
-	if (!IS_ERR(pll_ref))
-		mout_audss_p[0] = __clk_get_name(pll_ref);
-	if (!IS_ERR(pll_in))
-		mout_audss_p[1] = __clk_get_name(pll_in);
+	for (i = 0; i < ARRAY_SIZE(clk_name_ref); i++) {
+		tmp = devm_clk_get(&pdev->dev, clk_name_ref[i]);
+		if (!IS_ERR(tmp))
+			clk_name_actual[i] = __clk_get_name(tmp);
+	}
+
+	mout_audss_p[0] = clk_name_actual[0];
+	mout_audss_p[1] = clk_name_actual[1];
+	mout_i2s_p[1] = clk_name_actual[2];
+	mout_i2s_p[2] = clk_name_actual[3];
+	sclk_pcm_p = clk_name_actual[4];
+
 	clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
 				mout_audss_p, ARRAY_SIZE(mout_audss_p),
 				CLK_SET_RATE_NO_REPARENT,
 				reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
 
-	cdclk = devm_clk_get(&pdev->dev, "cdclk");
-	sclk_audio = devm_clk_get(&pdev->dev, "sclk_audio");
-	if (!IS_ERR(cdclk))
-		mout_i2s_p[1] = __clk_get_name(cdclk);
-	if (!IS_ERR(sclk_audio))
-		mout_i2s_p[2] = __clk_get_name(sclk_audio);
 	clk_table[EXYNOS_MOUT_I2S] = clk_register_mux(NULL, "mout_i2s",
 				mout_i2s_p, ARRAY_SIZE(mout_i2s_p),
 				CLK_SET_RATE_NO_REPARENT,
@@ -161,9 +165,6 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 				 "sclk_pcm", CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 4, 0, &lock);
 
-	sclk_pcm_in = devm_clk_get(&pdev->dev, "sclk_pcm_in");
-	if (!IS_ERR(sclk_pcm_in))
-		sclk_pcm_p = __clk_get_name(sclk_pcm_in);
 	clk_table[EXYNOS_SCLK_PCM] = clk_register_gate(NULL, "sclk_pcm",
 				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] clk: exynos-audss: Update as per existing framework
  2014-07-11  9:37 [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework Tushar Behera
  2014-07-11  9:37 ` [PATCH 1/2] clk: exynos-audss: Simplify code to get clock names Tushar Behera
@ 2014-07-11  9:37 ` Tushar Behera
  2014-07-11 10:40   ` Sylwester Nawrocki
  2014-07-11  9:51 ` [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework Tomasz Figa
  2 siblings, 1 reply; 7+ messages in thread
From: Tushar Behera @ 2014-07-11  9:37 UTC (permalink / raw)
  To: linux-arm-kernel

Change exynos-audss clock driver as per existing clock framework from
the existing module driver framework.

Signed-off-by: Tushar Behera <tushar.b@samsung.com>
---
 drivers/clk/samsung/clk-exynos-audss.c |  239 ++++++++++++--------------------
 1 file changed, 87 insertions(+), 152 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-audss.c b/drivers/clk/samsung/clk-exynos-audss.c
index ebfc5da..49f4163 100644
--- a/drivers/clk/samsung/clk-exynos-audss.c
+++ b/drivers/clk/samsung/clk-exynos-audss.c
@@ -19,26 +19,25 @@
 
 #include <dt-bindings/clock/exynos-audss-clk.h>
 
+#include "clk.h"
+
 enum exynos_audss_clk_type {
 	TYPE_EXYNOS4210,
 	TYPE_EXYNOS5250,
 	TYPE_EXYNOS5420,
 };
 
-static DEFINE_SPINLOCK(lock);
-static struct clk **clk_table;
 static void __iomem *reg_base;
-static struct clk_onecell_data clk_data;
 
-#define ASS_CLK_SRC 0x0
-#define ASS_CLK_DIV 0x4
-#define ASS_CLK_GATE 0x8
+#define ASS_CLK_SRC	0x0
+#define ASS_CLK_DIV	0x4
+#define ASS_CLK_GATE	0x8
 
 #ifdef CONFIG_PM_SLEEP
 static unsigned long reg_save[][2] = {
-	{ASS_CLK_SRC,  0},
-	{ASS_CLK_DIV,  0},
-	{ASS_CLK_GATE, 0},
+	{ASS_CLK_SRC,	0},
+	{ASS_CLK_DIV,	0},
+	{ASS_CLK_GATE,	0},
 };
 
 static int exynos_audss_clk_suspend(void)
@@ -65,21 +64,10 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 };
 #endif /* CONFIG_PM_SLEEP */
 
-static const struct of_device_id exynos_audss_clk_of_match[] = {
-	{ .compatible = "samsung,exynos4210-audss-clock",
-	  .data = (void *)TYPE_EXYNOS4210, },
-	{ .compatible = "samsung,exynos5250-audss-clock",
-	  .data = (void *)TYPE_EXYNOS5250, },
-	{ .compatible = "samsung,exynos5420-audss-clock",
-	  .data = (void *)TYPE_EXYNOS5420, },
-	{},
-};
-
-/* register exynos_audss clocks */
-static int exynos_audss_clk_probe(struct platform_device *pdev)
+static void __init exynos_audss_clk_init(struct device_node *np,
+				enum exynos_audss_clk_type variant)
 {
-	int i, ret = 0;
-	struct resource *res;
+	int i;
 	struct clk *tmp;
 	const char *clk_name_ref[] = {
 		"pll_ref", "pll_in", "cdclk", "sclk_audio", "sclk_pcm_in" };
@@ -87,163 +75,110 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 		"fin_pll", "fout_epll", "cdclk0", "sclk_audio0", "sclk_pcm0"};
 	const char *mout_audss_p[] = {"fin_pll", "fout_epll"};
 	const char *mout_i2s_p[] = {"mout_audss", "cdclk0", "sclk_audio0"};
-	const char *sclk_pcm_p = "sclk_pcm0";
-	const struct of_device_id *match;
-	enum exynos_audss_clk_type variant;
-
-	match = of_match_node(exynos_audss_clk_of_match, pdev->dev.of_node);
-	if (!match)
-		return -EINVAL;
-	variant = (enum exynos_audss_clk_type)match->data;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(reg_base)) {
-		dev_err(&pdev->dev, "failed to map audss registers\n");
-		return PTR_ERR(reg_base);
-	}
 
-	clk_table = devm_kzalloc(&pdev->dev,
-				sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
-				GFP_KERNEL);
-	if (!clk_table)
-		return -ENOMEM;
-
-	clk_data.clks = clk_table;
-	if (variant == TYPE_EXYNOS5420)
-		clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
-	else
-		clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS - 1;
+	char sclk_pcm_p[32];
+
+	struct samsung_mux_clock exynos_audss_mux_clks[] = {
+		MUX(EXYNOS_MOUT_AUDSS, "mout_audss", mout_audss_p,
+				ASS_CLK_SRC, 0, 1),
+		MUX(EXYNOS_MOUT_I2S, "mout_i2s", mout_i2s_p,
+				ASS_CLK_SRC, 2, 2),
+	};
+
+	struct samsung_div_clock exynos_audss_div_clks[] = {
+		DIV(EXYNOS_DOUT_SRP, "dout_srp", "mout_audss",
+				ASS_CLK_DIV, 0, 4),
+		DIV(EXYNOS_DOUT_AUD_BUS, "dout_aud_bus", "dout_srp",
+				ASS_CLK_DIV, 4, 4),
+		DIV(EXYNOS_DOUT_I2S, "dout_i2s", "mout_i2s", ASS_CLK_DIV, 8, 4),
+	};
+
+	struct samsung_gate_clock exynos_audss_gate_clks[] = {
+		GATE(EXYNOS_SRP_CLK, "srp_clk", "dout_srp",
+				ASS_CLK_GATE, 0, CLK_SET_RATE_PARENT, 0),
+		GATE(EXYNOS_I2S_BUS, "i2s_bus", "dout_aud_bus",
+				ASS_CLK_GATE, 2, CLK_SET_RATE_PARENT, 0),
+		GATE(EXYNOS_SCLK_I2S, "sclk_i2s", "dout_i2s",
+				ASS_CLK_GATE, 3, CLK_SET_RATE_PARENT, 0),
+		GATE(EXYNOS_PCM_BUS, "pcm_bus", "sclk_pcm",
+				ASS_CLK_GATE, 4, CLK_SET_RATE_PARENT, 0),
+		GATE(EXYNOS_SCLK_PCM, "sclk_pcm", sclk_pcm_p,
+				ASS_CLK_GATE, 5, CLK_SET_RATE_PARENT, 0),
+	};
+
+	struct samsung_gate_clock exynos5420_audss_gate_clks[] = {
+		GATE(EXYNOS_ADMA, "adma", "dout_srp",
+				ASS_CLK_GATE, 9, CLK_SET_RATE_PARENT, 0),
+	};
+
+	struct samsung_clk_provider *ctx;
+	int nr_clks = (variant == TYPE_EXYNOS5420) ?
+			EXYNOS_AUDSS_MAX_CLKS : EXYNOS_AUDSS_MAX_CLKS - 1;
+
+	if (!np)
+		panic("%s: failed to get device node for clock provider\n",
+				__func__);
+
+	reg_base = of_iomap(np, 0);
+	if (!reg_base)
+		panic("%s: failed to map registers\n", __func__);
+
+	ctx = samsung_clk_init(np, reg_base, nr_clks);
+	if (!ctx)
+		panic("%s: failed to get clock provier context\n", __func__);
 
 	for (i = 0; i < ARRAY_SIZE(clk_name_ref); i++) {
-		tmp = devm_clk_get(&pdev->dev, clk_name_ref[i]);
-		if (!IS_ERR(tmp))
+		tmp = of_clk_get_by_name(np, clk_name_ref[i]);
+		if (!IS_ERR(tmp)) {
 			clk_name_actual[i] = __clk_get_name(tmp);
+			clk_put(tmp);
+		}
 	}
 
 	mout_audss_p[0] = clk_name_actual[0];
 	mout_audss_p[1] = clk_name_actual[1];
 	mout_i2s_p[1] = clk_name_actual[2];
 	mout_i2s_p[2] = clk_name_actual[3];
-	sclk_pcm_p = clk_name_actual[4];
-
-	clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
-				mout_audss_p, ARRAY_SIZE(mout_audss_p),
-				CLK_SET_RATE_NO_REPARENT,
-				reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
-
-	clk_table[EXYNOS_MOUT_I2S] = clk_register_mux(NULL, "mout_i2s",
-				mout_i2s_p, ARRAY_SIZE(mout_i2s_p),
-				CLK_SET_RATE_NO_REPARENT,
-				reg_base + ASS_CLK_SRC, 2, 2, 0, &lock);
+	strncpy(sclk_pcm_p, clk_name_actual[4], strlen(clk_name_actual[4]));
 
-	clk_table[EXYNOS_DOUT_SRP] = clk_register_divider(NULL, "dout_srp",
-				"mout_audss", 0, reg_base + ASS_CLK_DIV, 0, 4,
-				0, &lock);
+	samsung_clk_register_mux(ctx, exynos_audss_mux_clks,
+				ARRAY_SIZE(exynos_audss_mux_clks));
 
-	clk_table[EXYNOS_DOUT_AUD_BUS] = clk_register_divider(NULL,
-				"dout_aud_bus", "dout_srp", 0,
-				reg_base + ASS_CLK_DIV, 4, 4, 0, &lock);
+	samsung_clk_register_div(ctx, exynos_audss_div_clks,
+				ARRAY_SIZE(exynos_audss_div_clks));
 
-	clk_table[EXYNOS_DOUT_I2S] = clk_register_divider(NULL, "dout_i2s",
-				"mout_i2s", 0, reg_base + ASS_CLK_DIV, 8, 4, 0,
-				&lock);
-
-	clk_table[EXYNOS_SRP_CLK] = clk_register_gate(NULL, "srp_clk",
-				"dout_srp", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 0, 0, &lock);
-
-	clk_table[EXYNOS_I2S_BUS] = clk_register_gate(NULL, "i2s_bus",
-				"dout_aud_bus", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 2, 0, &lock);
-
-	clk_table[EXYNOS_SCLK_I2S] = clk_register_gate(NULL, "sclk_i2s",
-				"dout_i2s", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 3, 0, &lock);
-
-	clk_table[EXYNOS_PCM_BUS] = clk_register_gate(NULL, "pcm_bus",
-				 "sclk_pcm", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 4, 0, &lock);
-
-	clk_table[EXYNOS_SCLK_PCM] = clk_register_gate(NULL, "sclk_pcm",
-				sclk_pcm_p, CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 5, 0, &lock);
+	samsung_clk_register_gate(ctx, exynos_audss_gate_clks,
+				ARRAY_SIZE(exynos_audss_gate_clks));
 
 	if (variant == TYPE_EXYNOS5420) {
-		clk_table[EXYNOS_ADMA] = clk_register_gate(NULL, "adma",
-				"dout_srp", CLK_SET_RATE_PARENT,
-				reg_base + ASS_CLK_GATE, 9, 0, &lock);
-	}
-
-	for (i = 0; i < clk_data.clk_num; i++) {
-		if (IS_ERR(clk_table[i])) {
-			dev_err(&pdev->dev, "failed to register clock %d\n", i);
-			ret = PTR_ERR(clk_table[i]);
-			goto unregister;
-		}
-	}
-
-	ret = of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
-					&clk_data);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to add clock provider\n");
-		goto unregister;
+		samsung_clk_register_gate(ctx, exynos5420_audss_gate_clks,
+					ARRAY_SIZE(exynos5420_audss_gate_clks));
 	}
 
 #ifdef CONFIG_PM_SLEEP
 	register_syscore_ops(&exynos_audss_clk_syscore_ops);
 #endif
 
-	dev_info(&pdev->dev, "setup completed\n");
-
-	return 0;
-
-unregister:
-	for (i = 0; i < clk_data.clk_num; i++) {
-		if (!IS_ERR(clk_table[i]))
-			clk_unregister(clk_table[i]);
-	}
-
-	return ret;
+	pr_info("%s setup completed\n", __func__);
 }
 
-static int exynos_audss_clk_remove(struct platform_device *pdev)
+static void exynos4210_audss_clk_init(struct device_node *np)
 {
-	int i;
-
-	of_clk_del_provider(pdev->dev.of_node);
-
-	for (i = 0; i < clk_data.clk_num; i++) {
-		if (!IS_ERR(clk_table[i]))
-			clk_unregister(clk_table[i]);
-	}
-
-	return 0;
+	exynos_audss_clk_init(np, TYPE_EXYNOS4210);
 }
+CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
+		exynos4210_audss_clk_init);
 
-static struct platform_driver exynos_audss_clk_driver = {
-	.driver	= {
-		.name = "exynos-audss-clk",
-		.owner = THIS_MODULE,
-		.of_match_table = exynos_audss_clk_of_match,
-	},
-	.probe = exynos_audss_clk_probe,
-	.remove = exynos_audss_clk_remove,
-};
-
-static int __init exynos_audss_clk_init(void)
+static void exynos5250_audss_clk_init(struct device_node *np)
 {
-	return platform_driver_register(&exynos_audss_clk_driver);
+	exynos_audss_clk_init(np, TYPE_EXYNOS5250);
 }
-core_initcall(exynos_audss_clk_init);
+CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
+		exynos5250_audss_clk_init);
 
-static void __exit exynos_audss_clk_exit(void)
+static void exynos5420_audss_clk_init(struct device_node *np)
 {
-	platform_driver_unregister(&exynos_audss_clk_driver);
+	exynos_audss_clk_init(np, TYPE_EXYNOS5420);
 }
-module_exit(exynos_audss_clk_exit);
-
-MODULE_AUTHOR("Padmavathi Venna <padma.v@samsung.com>");
-MODULE_DESCRIPTION("Exynos Audio Subsystem Clock Controller");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:exynos-audss-clk");
+CLK_OF_DECLARE(exynos5420_audss_clk, "samsung,exynos5420-audss-clock",
+		exynos5420_audss_clk_init);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework
  2014-07-11  9:37 [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework Tushar Behera
  2014-07-11  9:37 ` [PATCH 1/2] clk: exynos-audss: Simplify code to get clock names Tushar Behera
  2014-07-11  9:37 ` [PATCH 2/2] clk: exynos-audss: Update as per existing framework Tushar Behera
@ 2014-07-11  9:51 ` Tomasz Figa
  2014-07-11 10:09   ` Tushar Behera
  2 siblings, 1 reply; 7+ messages in thread
From: Tomasz Figa @ 2014-07-11  9:51 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Tushar,

On 11.07.2014 11:37, Tushar Behera wrote:
> The patchset is targetted as moving exising exynos-audss clock driver from being
> a module driver. The driver is now registered through CLK_OF_DECLARE and is
> inline with other Samsung clock drivers.

I'm afraid I have to NAK this series or at least the part converting the
driver back to use CLK_OF_DECLARE().

We have deliberately made this driver a platform driver, because this is
how drivers should be modeled in Linux kernel whenever possible.
CLK_OF_DECLARE() should be only considered a hack to work around late
initialization of driver model. Reverting this change without a good
reason (and you haven't provided such) is just going backwards.

Rest of this series is actually quite nice, though, as reusing Samsung
clock helpers reduces the line count significantly, so if you could
rework this to keep this driver a platform driver then we could get
something I could ack.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework
  2014-07-11  9:51 ` [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework Tomasz Figa
@ 2014-07-11 10:09   ` Tushar Behera
  0 siblings, 0 replies; 7+ messages in thread
From: Tushar Behera @ 2014-07-11 10:09 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2014 03:21 PM, Tomasz Figa wrote:
> Hi Tushar,
> 
> On 11.07.2014 11:37, Tushar Behera wrote:
>> The patchset is targetted as moving exising exynos-audss clock driver from being
>> a module driver. The driver is now registered through CLK_OF_DECLARE and is
>> inline with other Samsung clock drivers.
> 
> I'm afraid I have to NAK this series or at least the part converting the
> driver back to use CLK_OF_DECLARE().
> 

Considering the more prevalent usage of CLK_OF_DECLARE() in drivers/clk
led me into thinking it was the normal way for the clock drivers.
Keeping only one clock driver using a different approach looked odd to
me. Anyways, I don't have any other reason to pursue this case.

> We have deliberately made this driver a platform driver, because this is
> how drivers should be modeled in Linux kernel whenever possible.
> CLK_OF_DECLARE() should be only considered a hack to work around late
> initialization of driver model. Reverting this change without a good
> reason (and you haven't provided such) is just going backwards.
> 
> Rest of this series is actually quite nice, though, as reusing Samsung
> clock helpers reduces the line count significantly, so if you could
> rework this to keep this driver a platform driver then we could get
> something I could ack.
> 

Sure. There are still some valid cleanups even if we plan to retain the
platform driver infrastructure. I will split and repost.

> Best regards,
> Tomasz
> 


-- 
Tushar Behera

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] clk: exynos-audss: Update as per existing framework
  2014-07-11  9:37 ` [PATCH 2/2] clk: exynos-audss: Update as per existing framework Tushar Behera
@ 2014-07-11 10:40   ` Sylwester Nawrocki
  2014-07-11 12:07     ` Tushar Behera
  0 siblings, 1 reply; 7+ messages in thread
From: Sylwester Nawrocki @ 2014-07-11 10:40 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Tushar,

On 11/07/14 11:37, Tushar Behera wrote:
> Change exynos-audss clock driver as per existing clock framework from
> the existing module driver framework.

Can you explain what's the actual issue you're trying to solve with that
patch ? What's the problem with this driver being a platform driver ?
It feels we're moving in circles here, see

commit b37a4224104568198b93fb9831224cfe7d83fff8
Author: Andrew Bresticker <abrestic@chromium.org>
Date:   Wed Sep 25 14:12:47 2013 -0700

    clk: exynos-audss: convert to platform device

    The Exynos AudioSS clock controller will later be modified to allow
    input clocks to be specified via device-tree in order to support
    multiple Exynos SoCs.  This will introduce a dependency on the core
    SoC clock controller being initialized first so that the AudioSS driver
    can look up its input clocks, but the order in which clock providers
    are probed in of_clk_init() is not guaranteed.  Since deferred probing
    is not supported in of_clk_init() and the AudioSS block is not the core
    controller, we can initialize it later as a platform device.

    Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
    Acked-by: Tomasz Figa <t.figa@samsung.com>
    Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
    Acked-by: Mike Turquette <mturquette@linaro.org>
    Acked-by: Kukjin Kim <kgene.kim@samsung.com>
    Signed-off-by: Tomasz Figa <t.figa@samsung.com>

I realize of_clk_init() now handles better clock provider dependencies,
nevertheless do we really need all this churn ?

--
Thanks,
Sylwester

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] clk: exynos-audss: Update as per existing framework
  2014-07-11 10:40   ` Sylwester Nawrocki
@ 2014-07-11 12:07     ` Tushar Behera
  0 siblings, 0 replies; 7+ messages in thread
From: Tushar Behera @ 2014-07-11 12:07 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/11/2014 04:10 PM, Sylwester Nawrocki wrote:
> Hi Tushar,
> 
> On 11/07/14 11:37, Tushar Behera wrote:
>> Change exynos-audss clock driver as per existing clock framework from
>> the existing module driver framework.
> 
> Can you explain what's the actual issue you're trying to solve with that
> patch ? What's the problem with this driver being a platform driver ?
> It feels we're moving in circles here, see
> 
> commit b37a4224104568198b93fb9831224cfe7d83fff8
> Author: Andrew Bresticker <abrestic@chromium.org>
> Date:   Wed Sep 25 14:12:47 2013 -0700
> 
>     clk: exynos-audss: convert to platform device
> 
>     The Exynos AudioSS clock controller will later be modified to allow
>     input clocks to be specified via device-tree in order to support
>     multiple Exynos SoCs.  This will introduce a dependency on the core
>     SoC clock controller being initialized first so that the AudioSS driver
>     can look up its input clocks, but the order in which clock providers
>     are probed in of_clk_init() is not guaranteed.  Since deferred probing
>     is not supported in of_clk_init() and the AudioSS block is not the core
>     controller, we can initialize it later as a platform device.
> 
>     Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>     Acked-by: Tomasz Figa <t.figa@samsung.com>
>     Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>     Acked-by: Mike Turquette <mturquette@linaro.org>
>     Acked-by: Kukjin Kim <kgene.kim@samsung.com>
>     Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> 
> I realize of_clk_init() now handles better clock provider dependencies,
> nevertheless do we really need all this churn ?
> 

I have sent v2 of the patchset while retaining the platform driver
infrastructure. Awaiting your review comments on that.

> --
> Thanks,
> Sylwester
> 

Thanks,
-- 
Tushar Behera

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-07-11 12:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-11  9:37 [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework Tushar Behera
2014-07-11  9:37 ` [PATCH 1/2] clk: exynos-audss: Simplify code to get clock names Tushar Behera
2014-07-11  9:37 ` [PATCH 2/2] clk: exynos-audss: Update as per existing framework Tushar Behera
2014-07-11 10:40   ` Sylwester Nawrocki
2014-07-11 12:07     ` Tushar Behera
2014-07-11  9:51 ` [PATCH 0/2] clk: exynos-audss: Adapt to exising clock framework Tomasz Figa
2014-07-11 10:09   ` Tushar Behera

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).