[PATCH V2 0/3] clk: Samsung: audss: Register audio subsytem clocks using common clk framework

[PATCH V2 0/3] clk: Samsung: audss: Register audio subsytem clocks using common clk framework

[Padmavathi Venna]

Samsung S5PV210 and Exynos SoC has a separate subsystem for audio. This subsystem
has a internal clock controller which controls i2s0 and pcm0 clocks. This patch
series adds the Samsung audio subsytem clock to the common clock framework and
provides the I2S controllers clock information in the dtsi file.

This patch series is dependent on 
https://patchwork.kernel.org/patch/2532091/
http://www.spinics.net/lists/linux-kbuild/msg07616.html

This patch series is made based on Kukjin Kim for-next branch

Changes since V1:
	- Reworked on all review comments by Sylwester Nawrocki
	- Added a header file for all clock indexes as requested by Sylwester
	- Added different compatible names for s5pv210, exynos4 and exynos5
	- Registered the pcm clocks with common clock framework
	
Padmavathi Venna (3):
  clk: samsung: register audio subsystem clocks using common clock
    framework
  ARM: dts: add Exynos audio subsystem clock controller node
  ARM: dts: add clock provider information for i2s controllers in
    Exynos5250

 .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  |   16 +++
 drivers/clk/samsung/Makefile                       |    1 +
 drivers/clk/samsung/clk-samsung-audss.c            |  137 ++++++++++++++++++++
 include/dt-bindings/clk/samsung-audss-clk.h        |   25 ++++
 5 files changed, 243 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
 create mode 100644 drivers/clk/samsung/clk-samsung-audss.c
 create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h

-- 
1.7.4.4

[PATCH V2 0/3] clk: Samsung: audss: Register audio subsytem clocks using common clk framework

[Padmavathi Venna]

Samsung S5PV210 and Exynos SoC has a separate subsystem for audio. This subsystem
has a internal clock controller which controls i2s0 and pcm0 clocks. This patch
series adds the Samsung audio subsytem clock to the common clock framework and
provides the I2S controllers clock information in the dtsi file.

This patch series is dependent on 
https://patchwork.kernel.org/patch/2532091/
http://www.spinics.net/lists/linux-kbuild/msg07616.html

This patch series is made based on Kukjin Kim for-next branch

Changes since V1:
	- Reworked on all review comments by Sylwester Nawrocki
	- Added a header file for all clock indexes as requested by Sylwester
	- Added different compatible names for s5pv210, exynos4 and exynos5
	- Registered the pcm clocks with common clock framework
	
Padmavathi Venna (3):
  clk: samsung: register audio subsystem clocks using common clock
    framework
  ARM: dts: add Exynos audio subsystem clock controller node
  ARM: dts: add clock provider information for i2s controllers in
    Exynos5250

 .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
 arch/arm/boot/dts/exynos5250.dtsi                  |   16 +++
 drivers/clk/samsung/Makefile                       |    1 +
 drivers/clk/samsung/clk-samsung-audss.c            |  137 ++++++++++++++++++++
 include/dt-bindings/clk/samsung-audss-clk.h        |   25 ++++
 5 files changed, 243 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
 create mode 100644 drivers/clk/samsung/clk-samsung-audss.c
 create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h

-- 
1.7.4.4

[PATCH V2 1/3] clk: samsung: register audio subsystem clocks using common clock framework

[Padmavathi Venna]

Audio subsystem is introduced in s5pv210 and exynos platforms.
This has seperate clock controller which can control i2s0 and
pcm0 clocks. This patch registers the audio subsystem clocks
with the common clock framework.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
 drivers/clk/samsung/Makefile                       |    1 +
 drivers/clk/samsung/clk-samsung-audss.c            |  137 ++++++++++++++++++++
 include/dt-bindings/clk/samsung-audss-clk.h        |   25 ++++
 4 files changed, 227 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
 create mode 100644 drivers/clk/samsung/clk-samsung-audss.c
 create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h

diff --git a/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
new file mode 100644
index 0000000..ec2cd0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
@@ -0,0 +1,64 @@
+* Samsung Audio Subsystem Clock Controller
+
+The Samsung Audio Subsystem clock controller generates and supplies clocks
+to Audio Subsystem block available in the S5PV210 and Exynos SoCs. The clock
+binding described here is applicable to all SoC's in the S5PV210 and Exynos
+family.
+
+Required Properties:
+
+- compatible: should be one of the following:
+  - "samsung,s5pv210-audss-clock" - controller compatible with all S5PV210 SoCs.
+  - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.
+  - "samsung,exynos5250-audss-clock" - controller compatible with all Exynos5 SoCs.
+
+- reg: physical base address and length of the controller's register set.
+
+- #clock-cells: should be 1.
+
+The following is the list of clocks generated by the controller. Each clock is
+assigned an identifier and client nodes use this identifier to specify the
+clock which they consume. Some of the clocks are available only on a particular
+Exynos4 SoC and this is specified where applicable.
+
+Provided clocks:
+
+Clock           ID      SoC (if specific)
+-----------------------------------------------
+
+mout_audss      0
+mout_i2s        1
+dout_srp        2
+dout_bus        3
+dout_i2s        4
+srp_clk         5
+i2s_bus         6
+sclk_i2s        7
+pcm_bus         8
+sclk_pcm        9
+
+Example 1: An example of a clock controller node is listed below.
+
+clock_audss: audss-clock-controller@03810000 {
+	compatible = "samsung,exynos5250-audss-clock";
+	reg = <0x03810000 0x0C>;
+	#clock-cells = <1>;
+};
+
+Example 2: I2S controller node that consumes the clock generated by the clock
+           controller. Refer to the standard clock bindings for information
+           about 'clocks' and 'clock-names' property.
+
+ i2s0: i2s@03830000 {
+		compatible = "samsung,i2s-v5";
+		reg = <0x03830000 0x100>;
+		dmas = <&pdma0 10
+			&pdma0 9
+			&pdma0 8>;
+		dma-names = "tx", "rx", "tx-sec";
+		clocks = <&clock_audss SAMSUNG_I2S_BUS>,
+			<&clock_audss SAMSUNG_I2S_BUS>,
+			<&clock_audss SAMSUNG_SCLK_I2S>;
+		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
+};
+
diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index b7c232e..5425fa8 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o
 obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4.o
 obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
 obj-$(CONFIG_SOC_EXYNOS5440)	+= clk-exynos5440.o
+obj-$(CONFIG_PLAT_SAMSUNG)	+= clk-samsung-audss.o
diff --git a/drivers/clk/samsung/clk-samsung-audss.c b/drivers/clk/samsung/clk-samsung-audss.c
new file mode 100644
index 0000000..97526b7
--- /dev/null
+++ b/drivers/clk/samsung/clk-samsung-audss.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * Author: Padmavathi Venna <padma.v@samsung.com>
+ *
+ * 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.
+ *
+ * Common Clock Framework support for Audio Subsystem Clock Controller.
+*/
+
+#include <linux/clkdev.h>
+#include <linux/io.h>
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/syscore_ops.h>
+
+#include <dt-bindings/clk/samsung-audss-clk.h>
+
+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
+
+static unsigned long reg_save[][2] = {
+	{ASS_CLK_SRC,  0},
+	{ASS_CLK_DIV,  0},
+	{ASS_CLK_GATE, 0},
+};
+
+/* list of all parent clock list */
+static const char *mout_audss_p[] = { "fin_pll", "fout_epll" };
+static const char *mout_i2s_p[] = { "mout_audss", "cdclk0", "sclk_audio0" };
+
+#ifdef CONFIG_PM_SLEEP
+static int samsung_audss_clk_suspend(void)
+{
+	int i;
+
+	for (i = 0; i < 3; i++)
+		reg_save[i][1] = readl(reg_base + reg_save[i][0]);
+
+	return 0;
+}
+
+static void samsung_audss_clk_resume(void)
+{
+	int i;
+
+	for (i = 0; i < 3; i++)
+		writel(reg_save[i][1], reg_base + reg_save[i][0]);
+}
+
+static struct syscore_ops samsung_audss_clk_syscore_ops = {
+	.suspend	= samsung_audss_clk_suspend,
+	.resume		= samsung_audss_clk_resume,
+};
+#endif /* CONFIG_PM_SLEEP */
+
+/* register samsung_audss clocks */
+void __init samsung_audss_clk_init(struct device_node *np)
+{
+	reg_base = of_iomap(np, 0);
+	if (!reg_base) {
+		pr_err("%s: failed to map audss registers\n", __func__);
+		return;
+	}
+
+	clk_table = kzalloc(sizeof(struct clk *) * SAMSUNG_AUDSS_MAX_CLKS,
+				GFP_KERNEL);
+	if (!clk_table) {
+		pr_err("%s: could not allocate clock lookup table\n", __func__);
+		return;
+	}
+
+	clk_data.clks = clk_table;
+	clk_data.clk_num = SAMSUNG_AUDSS_MAX_CLKS;
+	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+
+	clk_table[SAMSUNG_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
+				mout_audss_p, ARRAY_SIZE(mout_audss_p), 0,
+				reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
+	clk_register_clkdev(clk_table[SAMSUNG_MOUT_AUDSS], "mout_audss", NULL);
+
+	clk_table[SAMSUNG_MOUT_I2S] = clk_register_mux(NULL, "mout_i2s",
+				mout_i2s_p, ARRAY_SIZE(mout_i2s_p), 0,
+				reg_base + ASS_CLK_SRC, 2, 2, 0, &lock);
+	clk_register_clkdev(clk_table[SAMSUNG_MOUT_I2S], "mout_i2s", NULL);
+
+	clk_table[SAMSUNG_DOUT_SRP] = clk_register_divider(NULL, "dout_srp",
+				"mout_audss", 0, reg_base + ASS_CLK_DIV, 0, 4,
+				0, &lock);
+
+	clk_table[SAMSUNG_DOUT_BUS] = clk_register_divider(NULL, "dout_bus",
+				"dout_srp", 0, reg_base + ASS_CLK_DIV, 4, 4, 0,
+				&lock);
+
+	clk_table[SAMSUNG_DOUT_I2S] = clk_register_divider(NULL, "dout_i2s",
+				"mout_i2s", 0, reg_base + ASS_CLK_DIV, 8, 4, 0,
+				&lock);
+
+	clk_table[SAMSUNG_SRP_CLK] = clk_register_gate(NULL, "srp_clk",
+				"dout_srp", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 0, 0, &lock);
+
+	clk_table[SAMSUNG_I2S_BUS] = clk_register_gate(NULL, "i2s_bus",
+				"dout_bus", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 2, 0, &lock);
+
+	clk_table[SAMSUNG_SCLK_I2S] = clk_register_gate(NULL, "sclk_i2s",
+				"dout_i2s", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 3, 0, &lock);
+
+	clk_table[SAMSUNG_PCM_BUS] = clk_register_gate(NULL, "pcm_bus",
+				 "sclk_pcm", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 4, 0, &lock);
+
+	clk_table[SAMSUNG_SCLK_PCM] = clk_register_gate(NULL, "sclk_pcm",
+				"div_pcm0", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 5, 0, &lock);
+
+#ifdef CONFIG_PM_SLEEP
+	register_syscore_ops(&samsung_audss_clk_syscore_ops);
+#endif
+
+	pr_info("Exynos: Audss: clock setup completed\n");
+}
+CLK_OF_DECLARE(s5pv210_audss_clk, "samsung,s5pv210-audss-clock",
+		samsung_audss_clk_init);
+CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
+		samsung_audss_clk_init);
+CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
+		samsung_audss_clk_init);
diff --git a/include/dt-bindings/clk/samsung-audss-clk.h b/include/dt-bindings/clk/samsung-audss-clk.h
new file mode 100644
index 0000000..d335555
--- /dev/null
+++ b/include/dt-bindings/clk/samsung-audss-clk.h
@@ -0,0 +1,25 @@
+/*
+ * This header provides constants for Samsung audio subsystem
+ * clock controller.
+ *
+ * The constants defined in this header are being used in dts
+ * and samsung audss driver.
+ */
+
+#ifndef _DT_BINDINGS_CLK_SAMSUNG_AUDSS_H
+#define _DT_BINDINGS_CLK_SAMSUNG_AUDSS_H
+
+#define SAMSUNG_MOUT_AUDSS	0
+#define SAMSUNG_MOUT_I2S	1
+#define SAMSUNG_DOUT_SRP	2
+#define SAMSUNG_DOUT_BUS	3
+#define SAMSUNG_DOUT_I2S	4
+#define SAMSUNG_SRP_CLK		5
+#define SAMSUNG_I2S_BUS		6
+#define SAMSUNG_SCLK_I2S	7
+#define SAMSUNG_PCM_BUS		8
+#define SAMSUNG_SCLK_PCM	9
+
+#define SAMSUNG_AUDSS_MAX_CLKS	10
+
+#endif
-- 
1.7.4.4

[PATCH V2 1/3] clk: samsung: register audio subsystem clocks using common clock framework

[Padmavathi Venna]

Audio subsystem is introduced in s5pv210 and exynos platforms.
This has seperate clock controller which can control i2s0 and
pcm0 clocks. This patch registers the audio subsystem clocks
with the common clock framework.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
 drivers/clk/samsung/Makefile                       |    1 +
 drivers/clk/samsung/clk-samsung-audss.c            |  137 ++++++++++++++++++++
 include/dt-bindings/clk/samsung-audss-clk.h        |   25 ++++
 4 files changed, 227 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
 create mode 100644 drivers/clk/samsung/clk-samsung-audss.c
 create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h

diff --git a/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
new file mode 100644
index 0000000..ec2cd0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
@@ -0,0 +1,64 @@
+* Samsung Audio Subsystem Clock Controller
+
+The Samsung Audio Subsystem clock controller generates and supplies clocks
+to Audio Subsystem block available in the S5PV210 and Exynos SoCs. The clock
+binding described here is applicable to all SoC's in the S5PV210 and Exynos
+family.
+
+Required Properties:
+
+- compatible: should be one of the following:
+  - "samsung,s5pv210-audss-clock" - controller compatible with all S5PV210 SoCs.
+  - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.
+  - "samsung,exynos5250-audss-clock" - controller compatible with all Exynos5 SoCs.
+
+- reg: physical base address and length of the controller's register set.
+
+- #clock-cells: should be 1.
+
+The following is the list of clocks generated by the controller. Each clock is
+assigned an identifier and client nodes use this identifier to specify the
+clock which they consume. Some of the clocks are available only on a particular
+Exynos4 SoC and this is specified where applicable.
+
+Provided clocks:
+
+Clock           ID      SoC (if specific)
+-----------------------------------------------
+
+mout_audss      0
+mout_i2s        1
+dout_srp        2
+dout_bus        3
+dout_i2s        4
+srp_clk         5
+i2s_bus         6
+sclk_i2s        7
+pcm_bus         8
+sclk_pcm        9
+
+Example 1: An example of a clock controller node is listed below.
+
+clock_audss: audss-clock-controller at 03810000 {
+	compatible = "samsung,exynos5250-audss-clock";
+	reg = <0x03810000 0x0C>;
+	#clock-cells = <1>;
+};
+
+Example 2: I2S controller node that consumes the clock generated by the clock
+           controller. Refer to the standard clock bindings for information
+           about 'clocks' and 'clock-names' property.
+
+ i2s0: i2s at 03830000 {
+		compatible = "samsung,i2s-v5";
+		reg = <0x03830000 0x100>;
+		dmas = <&pdma0 10
+			&pdma0 9
+			&pdma0 8>;
+		dma-names = "tx", "rx", "tx-sec";
+		clocks = <&clock_audss SAMSUNG_I2S_BUS>,
+			<&clock_audss SAMSUNG_I2S_BUS>,
+			<&clock_audss SAMSUNG_SCLK_I2S>;
+		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
+};
+
diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index b7c232e..5425fa8 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o
 obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4.o
 obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
 obj-$(CONFIG_SOC_EXYNOS5440)	+= clk-exynos5440.o
+obj-$(CONFIG_PLAT_SAMSUNG)	+= clk-samsung-audss.o
diff --git a/drivers/clk/samsung/clk-samsung-audss.c b/drivers/clk/samsung/clk-samsung-audss.c
new file mode 100644
index 0000000..97526b7
--- /dev/null
+++ b/drivers/clk/samsung/clk-samsung-audss.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * Author: Padmavathi Venna <padma.v@samsung.com>
+ *
+ * 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.
+ *
+ * Common Clock Framework support for Audio Subsystem Clock Controller.
+*/
+
+#include <linux/clkdev.h>
+#include <linux/io.h>
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/syscore_ops.h>
+
+#include <dt-bindings/clk/samsung-audss-clk.h>
+
+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
+
+static unsigned long reg_save[][2] = {
+	{ASS_CLK_SRC,  0},
+	{ASS_CLK_DIV,  0},
+	{ASS_CLK_GATE, 0},
+};
+
+/* list of all parent clock list */
+static const char *mout_audss_p[] = { "fin_pll", "fout_epll" };
+static const char *mout_i2s_p[] = { "mout_audss", "cdclk0", "sclk_audio0" };
+
+#ifdef CONFIG_PM_SLEEP
+static int samsung_audss_clk_suspend(void)
+{
+	int i;
+
+	for (i = 0; i < 3; i++)
+		reg_save[i][1] = readl(reg_base + reg_save[i][0]);
+
+	return 0;
+}
+
+static void samsung_audss_clk_resume(void)
+{
+	int i;
+
+	for (i = 0; i < 3; i++)
+		writel(reg_save[i][1], reg_base + reg_save[i][0]);
+}
+
+static struct syscore_ops samsung_audss_clk_syscore_ops = {
+	.suspend	= samsung_audss_clk_suspend,
+	.resume		= samsung_audss_clk_resume,
+};
+#endif /* CONFIG_PM_SLEEP */
+
+/* register samsung_audss clocks */
+void __init samsung_audss_clk_init(struct device_node *np)
+{
+	reg_base = of_iomap(np, 0);
+	if (!reg_base) {
+		pr_err("%s: failed to map audss registers\n", __func__);
+		return;
+	}
+
+	clk_table = kzalloc(sizeof(struct clk *) * SAMSUNG_AUDSS_MAX_CLKS,
+				GFP_KERNEL);
+	if (!clk_table) {
+		pr_err("%s: could not allocate clock lookup table\n", __func__);
+		return;
+	}
+
+	clk_data.clks = clk_table;
+	clk_data.clk_num = SAMSUNG_AUDSS_MAX_CLKS;
+	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+
+	clk_table[SAMSUNG_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
+				mout_audss_p, ARRAY_SIZE(mout_audss_p), 0,
+				reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
+	clk_register_clkdev(clk_table[SAMSUNG_MOUT_AUDSS], "mout_audss", NULL);
+
+	clk_table[SAMSUNG_MOUT_I2S] = clk_register_mux(NULL, "mout_i2s",
+				mout_i2s_p, ARRAY_SIZE(mout_i2s_p), 0,
+				reg_base + ASS_CLK_SRC, 2, 2, 0, &lock);
+	clk_register_clkdev(clk_table[SAMSUNG_MOUT_I2S], "mout_i2s", NULL);
+
+	clk_table[SAMSUNG_DOUT_SRP] = clk_register_divider(NULL, "dout_srp",
+				"mout_audss", 0, reg_base + ASS_CLK_DIV, 0, 4,
+				0, &lock);
+
+	clk_table[SAMSUNG_DOUT_BUS] = clk_register_divider(NULL, "dout_bus",
+				"dout_srp", 0, reg_base + ASS_CLK_DIV, 4, 4, 0,
+				&lock);
+
+	clk_table[SAMSUNG_DOUT_I2S] = clk_register_divider(NULL, "dout_i2s",
+				"mout_i2s", 0, reg_base + ASS_CLK_DIV, 8, 4, 0,
+				&lock);
+
+	clk_table[SAMSUNG_SRP_CLK] = clk_register_gate(NULL, "srp_clk",
+				"dout_srp", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 0, 0, &lock);
+
+	clk_table[SAMSUNG_I2S_BUS] = clk_register_gate(NULL, "i2s_bus",
+				"dout_bus", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 2, 0, &lock);
+
+	clk_table[SAMSUNG_SCLK_I2S] = clk_register_gate(NULL, "sclk_i2s",
+				"dout_i2s", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 3, 0, &lock);
+
+	clk_table[SAMSUNG_PCM_BUS] = clk_register_gate(NULL, "pcm_bus",
+				 "sclk_pcm", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 4, 0, &lock);
+
+	clk_table[SAMSUNG_SCLK_PCM] = clk_register_gate(NULL, "sclk_pcm",
+				"div_pcm0", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 5, 0, &lock);
+
+#ifdef CONFIG_PM_SLEEP
+	register_syscore_ops(&samsung_audss_clk_syscore_ops);
+#endif
+
+	pr_info("Exynos: Audss: clock setup completed\n");
+}
+CLK_OF_DECLARE(s5pv210_audss_clk, "samsung,s5pv210-audss-clock",
+		samsung_audss_clk_init);
+CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
+		samsung_audss_clk_init);
+CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
+		samsung_audss_clk_init);
diff --git a/include/dt-bindings/clk/samsung-audss-clk.h b/include/dt-bindings/clk/samsung-audss-clk.h
new file mode 100644
index 0000000..d335555
--- /dev/null
+++ b/include/dt-bindings/clk/samsung-audss-clk.h
@@ -0,0 +1,25 @@
+/*
+ * This header provides constants for Samsung audio subsystem
+ * clock controller.
+ *
+ * The constants defined in this header are being used in dts
+ * and samsung audss driver.
+ */
+
+#ifndef _DT_BINDINGS_CLK_SAMSUNG_AUDSS_H
+#define _DT_BINDINGS_CLK_SAMSUNG_AUDSS_H
+
+#define SAMSUNG_MOUT_AUDSS	0
+#define SAMSUNG_MOUT_I2S	1
+#define SAMSUNG_DOUT_SRP	2
+#define SAMSUNG_DOUT_BUS	3
+#define SAMSUNG_DOUT_I2S	4
+#define SAMSUNG_SRP_CLK		5
+#define SAMSUNG_I2S_BUS		6
+#define SAMSUNG_SCLK_I2S	7
+#define SAMSUNG_PCM_BUS		8
+#define SAMSUNG_SCLK_PCM	9
+
+#define SAMSUNG_AUDSS_MAX_CLKS	10
+
+#endif
-- 
1.7.4.4

[PATCH V2 2/3] ARM: dts: add Exynos audio subsystem clock controller node

[Padmavathi Venna]

Audio subsystem introduced in s5pv210 and exynos platforms
which has a internal clock controller. This patch adds a node
for the same on exynos5250.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 36c9d8d..7026de0 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -72,6 +72,12 @@
 		#clock-cells = <1>;
 	};
 
+	clock_audss: audss-clock-controller@0x03810000 {
+		compatible = "samsung,exynos5250-audss-clock";
+		reg = <0x03810000 0x0C>;
+		#clock-cells = <1>;
+	};
+
 	gic:interrupt-controller@10481000 {
 		compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
 		#interrupt-cells = <3>;
-- 
1.7.4.4

[PATCH V2 2/3] ARM: dts: add Exynos audio subsystem clock controller node

[Padmavathi Venna]

Audio subsystem introduced in s5pv210 and exynos platforms
which has a internal clock controller. This patch adds a node
for the same on exynos5250.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 36c9d8d..7026de0 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -72,6 +72,12 @@
 		#clock-cells = <1>;
 	};
 
+	clock_audss: audss-clock-controller at 0x03810000 {
+		compatible = "samsung,exynos5250-audss-clock";
+		reg = <0x03810000 0x0C>;
+		#clock-cells = <1>;
+	};
+
 	gic:interrupt-controller at 10481000 {
 		compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
 		#interrupt-cells = <3>;
-- 
1.7.4.4

[PATCH V2 3/3] ARM: dts: add clock provider information for i2s controllers in Exynos5250

[Padmavathi Venna]

Add clock lookup information for i2s controllers on exynos5250 SoC.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 7026de0..13305b9 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -20,6 +20,8 @@
 #include "skeleton.dtsi"
 #include "exynos5250-pinctrl.dtsi"
 
+#include <dt-bindings/clk/samsung-audss-clk.h>
+
 / {
 	compatible = "samsung,exynos5250";
 	interrupt-parent = <&gic>;
@@ -457,6 +459,10 @@
 			&pdma0 9
 			&pdma0 8>;
 		dma-names = "tx", "rx", "tx-sec";
+		clocks = <&clock_audss SAMSUNG_I2S_BUS>,
+			<&clock_audss SAMSUNG_I2S_BUS>,
+			<&clock_audss SAMSUNG_SCLK_I2S>;
+		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
 		samsung,supports-6ch;
 		samsung,supports-rstclr;
 		samsung,supports-secdai;
@@ -471,6 +477,8 @@
 		dmas = <&pdma1 12
 			&pdma1 11>;
 		dma-names = "tx", "rx";
+		clocks = <&clock 307>;
+		clock-names = "iis";
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s1_bus>;
 	};
@@ -481,6 +489,8 @@
 		dmas = <&pdma0 12
 			&pdma0 11>;
 		dma-names = "tx", "rx";
+		clocks = <&clock 308>;
+		clock-names = "iis";
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s2_bus>;
 	};
-- 
1.7.4.4

[PATCH V2 3/3] ARM: dts: add clock provider information for i2s controllers in Exynos5250

[Padmavathi Venna]

Add clock lookup information for i2s controllers on exynos5250 SoC.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 7026de0..13305b9 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -20,6 +20,8 @@
 #include "skeleton.dtsi"
 #include "exynos5250-pinctrl.dtsi"
 
+#include <dt-bindings/clk/samsung-audss-clk.h>
+
 / {
 	compatible = "samsung,exynos5250";
 	interrupt-parent = <&gic>;
@@ -457,6 +459,10 @@
 			&pdma0 9
 			&pdma0 8>;
 		dma-names = "tx", "rx", "tx-sec";
+		clocks = <&clock_audss SAMSUNG_I2S_BUS>,
+			<&clock_audss SAMSUNG_I2S_BUS>,
+			<&clock_audss SAMSUNG_SCLK_I2S>;
+		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
 		samsung,supports-6ch;
 		samsung,supports-rstclr;
 		samsung,supports-secdai;
@@ -471,6 +477,8 @@
 		dmas = <&pdma1 12
 			&pdma1 11>;
 		dma-names = "tx", "rx";
+		clocks = <&clock 307>;
+		clock-names = "iis";
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s1_bus>;
 	};
@@ -481,6 +489,8 @@
 		dmas = <&pdma0 12
 			&pdma0 11>;
 		dma-names = "tx", "rx";
+		clocks = <&clock 308>;
+		clock-names = "iis";
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s2_bus>;
 	};
-- 
1.7.4.4

Re: [PATCH V2 2/3] ARM: dts: add Exynos audio subsystem clock controller node

[Tomasz Figa]

Hi Padmavathi,

On Tuesday 07 of May 2013 12:13:35 Padmavathi Venna wrote:
> Audio subsystem introduced in s5pv210 and exynos platforms
> which has a internal clock controller. This patch adds a node
> for the same on exynos5250.
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5250.dtsi |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi index 36c9d8d..7026de0 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -72,6 +72,12 @@
>  		#clock-cells = <1>;
>  	};
> 
> +	clock_audss: audss-clock-controller@0x03810000 {

Just a nitpick: this should be @3810000, without the 0x and leading zeroes.

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Kernel and System Framework

> +		compatible = "samsung,exynos5250-audss-clock";
> +		reg = <0x03810000 0x0C>;
> +		#clock-cells = <1>;
> +	};
> +
>  	gic:interrupt-controller@10481000 {
>  		compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>  		#interrupt-cells = <3>;

[PATCH V2 2/3] ARM: dts: add Exynos audio subsystem clock controller node

[Tomasz Figa]

Hi Padmavathi,

On Tuesday 07 of May 2013 12:13:35 Padmavathi Venna wrote:
> Audio subsystem introduced in s5pv210 and exynos platforms
> which has a internal clock controller. This patch adds a node
> for the same on exynos5250.
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  arch/arm/boot/dts/exynos5250.dtsi |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi index 36c9d8d..7026de0 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -72,6 +72,12 @@
>  		#clock-cells = <1>;
>  	};
> 
> +	clock_audss: audss-clock-controller at 0x03810000 {

Just a nitpick: this should be @3810000, without the 0x and leading zeroes.

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Kernel and System Framework

> +		compatible = "samsung,exynos5250-audss-clock";
> +		reg = <0x03810000 0x0C>;
> +		#clock-cells = <1>;
> +	};
> +
>  	gic:interrupt-controller at 10481000 {
>  		compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>  		#interrupt-cells = <3>;

Re: [PATCH V2 2/3] ARM: dts: add Exynos audio subsystem clock controller node

[Padma Venkat]

Hi Tomasz,

On Tue, May 7, 2013 at 3:24 PM, Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> Hi Padmavathi,
>
> On Tuesday 07 of May 2013 12:13:35 Padmavathi Venna wrote:
>> Audio subsystem introduced in s5pv210 and exynos platforms
>> which has a internal clock controller. This patch adds a node
>> for the same on exynos5250.
>>
>> Signed-off-by: Padmavathi Venna <padma.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  arch/arm/boot/dts/exynos5250.dtsi |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
>> b/arch/arm/boot/dts/exynos5250.dtsi index 36c9d8d..7026de0 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>> @@ -72,6 +72,12 @@
>>               #clock-cells = <1>;
>>       };
>>
>> +     clock_audss: audss-clock-controller@0x03810000 {
>
> Just a nitpick: this should be @3810000, without the 0x and leading zeroes.

Okey. I will change this.

>
> Best regards,
> --
> Tomasz Figa
> Samsung Poland R&D Center
> SW Solution Development, Kernel and System Framework
>
>> +             compatible = "samsung,exynos5250-audss-clock";
>> +             reg = <0x03810000 0x0C>;
>> +             #clock-cells = <1>;
>> +     };
>> +
>>       gic:interrupt-controller@10481000 {
>>               compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>>               #interrupt-cells = <3>;
>
Thanks
Padma

[PATCH V2 2/3] ARM: dts: add Exynos audio subsystem clock controller node

[Padma Venkat]

Hi Tomasz,

On Tue, May 7, 2013 at 3:24 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Padmavathi,
>
> On Tuesday 07 of May 2013 12:13:35 Padmavathi Venna wrote:
>> Audio subsystem introduced in s5pv210 and exynos platforms
>> which has a internal clock controller. This patch adds a node
>> for the same on exynos5250.
>>
>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
>> ---
>>  arch/arm/boot/dts/exynos5250.dtsi |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
>> b/arch/arm/boot/dts/exynos5250.dtsi index 36c9d8d..7026de0 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>> @@ -72,6 +72,12 @@
>>               #clock-cells = <1>;
>>       };
>>
>> +     clock_audss: audss-clock-controller at 0x03810000 {
>
> Just a nitpick: this should be @3810000, without the 0x and leading zeroes.

Okey. I will change this.

>
> Best regards,
> --
> Tomasz Figa
> Samsung Poland R&D Center
> SW Solution Development, Kernel and System Framework
>
>> +             compatible = "samsung,exynos5250-audss-clock";
>> +             reg = <0x03810000 0x0C>;
>> +             #clock-cells = <1>;
>> +     };
>> +
>>       gic:interrupt-controller at 10481000 {
>>               compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
>>               #interrupt-cells = <3>;
>
Thanks
Padma

Re: [PATCH V2 1/3] clk: samsung: register audio subsystem clocks using common clock framework

[Tomasz Figa]

Hi Padmavathi,

I managed to review the patch a bit more thoroughly and I had few more 
comments. You can find them inline.

On Tuesday 07 of May 2013 12:13:34 Padmavathi Venna wrote:
> Audio subsystem is introduced in s5pv210 and exynos platforms.
> This has seperate clock controller which can control i2s0 and
> pcm0 clocks. This patch registers the audio subsystem clocks
> with the common clock framework.
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
>  drivers/clk/samsung/Makefile                       |    1 +
>  drivers/clk/samsung/clk-samsung-audss.c            |  137
> ++++++++++++++++++++ include/dt-bindings/clk/samsung-audss-clk.h       
> |   25 ++++ 4 files changed, 227 insertions(+), 0 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/clock/clk-samsung-audss.txt create
> mode 100644 drivers/clk/samsung/clk-samsung-audss.c
>  create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h
> 
> diff --git
> a/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
> b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt new
> file mode 100644
> index 0000000..ec2cd0b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
> @@ -0,0 +1,64 @@
> +* Samsung Audio Subsystem Clock Controller
> +
> +The Samsung Audio Subsystem clock controller generates and supplies
> clocks +to Audio Subsystem block available in the S5PV210 and Exynos
> SoCs. The clock +binding described here is applicable to all SoC's in
> the S5PV210 and Exynos +family.
> +
> +Required Properties:
> +
> +- compatible: should be one of the following:
> +  - "samsung,s5pv210-audss-clock" - controller compatible with all
> S5PV210 SoCs. +  - "samsung,exynos4210-audss-clock" - controller
> compatible with all Exynos4 SoCs. +  - "samsung,exynos5250-audss-clock"
> - controller compatible with all Exynos5 SoCs. +
> +- reg: physical base address and length of the controller's register
> set. +
> +- #clock-cells: should be 1.
> +
> +The following is the list of clocks generated by the controller. Each
> clock is +assigned an identifier and client nodes use this identifier
> to specify the +clock which they consume. Some of the clocks are
> available only on a particular +Exynos4 SoC and this is specified where
> applicable.
> +
> +Provided clocks:
> +
> +Clock           ID      SoC (if specific)
> +-----------------------------------------------
> +
> +mout_audss      0
> +mout_i2s        1
> +dout_srp        2
> +dout_bus        3
> +dout_i2s        4
> +srp_clk         5
> +i2s_bus         6
> +sclk_i2s        7
> +pcm_bus         8
> +sclk_pcm        9
> +
> +Example 1: An example of a clock controller node is listed below.
> +
> +clock_audss: audss-clock-controller@03810000 {
> +	compatible = "samsung,exynos5250-audss-clock";
> +	reg = <0x03810000 0x0C>;
> +	#clock-cells = <1>;
> +};
> +
> +Example 2: I2S controller node that consumes the clock generated by the
> clock +           controller. Refer to the standard clock bindings for
> information +           about 'clocks' and 'clock-names' property.
> +
> + i2s0: i2s@03830000 {
> +		compatible = "samsung,i2s-v5";
> +		reg = <0x03830000 0x100>;
> +		dmas = <&pdma0 10
> +			&pdma0 9
> +			&pdma0 8>;
> +		dma-names = "tx", "rx", "tx-sec";
> +		clocks = <&clock_audss SAMSUNG_I2S_BUS>,
> +			<&clock_audss SAMSUNG_I2S_BUS>,
> +			<&clock_audss SAMSUNG_SCLK_I2S>;
> +		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
> +};
> +
> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
> index b7c232e..5425fa8 100644
> --- a/drivers/clk/samsung/Makefile
> +++ b/drivers/clk/samsung/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o
>  obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4.o
>  obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
>  obj-$(CONFIG_SOC_EXYNOS5440)	+= clk-exynos5440.o
> +obj-$(CONFIG_PLAT_SAMSUNG)	+= clk-samsung-audss.o
> diff --git a/drivers/clk/samsung/clk-samsung-audss.c
> b/drivers/clk/samsung/clk-samsung-audss.c new file mode 100644
> index 0000000..97526b7
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-samsung-audss.c
> @@ -0,0 +1,137 @@
> +/*
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * Author: Padmavathi Venna <padma.v@samsung.com>
> + *
> + * 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.
> + *
> + * Common Clock Framework support for Audio Subsystem Clock Controller.
> +*/
> +
> +#include <linux/clkdev.h>
> +#include <linux/io.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/syscore_ops.h>
> +
> +#include <dt-bindings/clk/samsung-audss-clk.h>
> +
> +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
> +
> +static unsigned long reg_save[][2] = {
> +	{ASS_CLK_SRC,  0},
> +	{ASS_CLK_DIV,  0},
> +	{ASS_CLK_GATE, 0},
> +};
> +
> +/* list of all parent clock list */
> +static const char *mout_audss_p[] = { "fin_pll", "fout_epll" };
> +static const char *mout_i2s_p[] = { "mout_audss", "cdclk0",
> "sclk_audio0" }; +
> +#ifdef CONFIG_PM_SLEEP
> +static int samsung_audss_clk_suspend(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < 3; i++)
> +		reg_save[i][1] = readl(reg_base + reg_save[i][0]);
> +
> +	return 0;
> +}
> +
> +static void samsung_audss_clk_resume(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < 3; i++)
> +		writel(reg_save[i][1], reg_base + reg_save[i][0]);
> +}
> +
> +static struct syscore_ops samsung_audss_clk_syscore_ops = {
> +	.suspend	= samsung_audss_clk_suspend,
> +	.resume		= samsung_audss_clk_resume,
> +};
> +#endif /* CONFIG_PM_SLEEP */
> +
> +/* register samsung_audss clocks */
> +void __init samsung_audss_clk_init(struct device_node *np)
> +{
> +	reg_base = of_iomap(np, 0);
> +	if (!reg_base) {
> +		pr_err("%s: failed to map audss registers\n", __func__);
> +		return;
> +	}
> +
> +	clk_table = kzalloc(sizeof(struct clk *) * SAMSUNG_AUDSS_MAX_CLKS,
> +				GFP_KERNEL);
> +	if (!clk_table) {
> +		pr_err("%s: could not allocate clock lookup table\n", 
__func__);
> +		return;
> +	}
> +
> +	clk_data.clks = clk_table;
> +	clk_data.clk_num = SAMSUNG_AUDSS_MAX_CLKS;
> +	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
> +
> +	clk_table[SAMSUNG_MOUT_AUDSS] = clk_register_mux(NULL, 
"mout_audss",
> +				mout_audss_p, ARRAY_SIZE(mout_audss_p), 0,
> +				reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
> +	clk_register_clkdev(clk_table[SAMSUNG_MOUT_AUDSS], "mout_audss",
> NULL); +

Is there are a reason for this clkdev lookup to be registered?

This driver seems to be used only in DT-case, so DT-based lookup should be 
enough.

> +	clk_table[SAMSUNG_MOUT_I2S] = clk_register_mux(NULL, "mout_i2s",
> +				mout_i2s_p, ARRAY_SIZE(mout_i2s_p), 0,
> +				reg_base + ASS_CLK_SRC, 2, 2, 0, &lock);
> +	clk_register_clkdev(clk_table[SAMSUNG_MOUT_I2S], "mout_i2s", 
NULL);

Same here.

> +
> +	clk_table[SAMSUNG_DOUT_SRP] = clk_register_divider(NULL, 
"dout_srp",
> +				"mout_audss", 0, reg_base + ASS_CLK_DIV, 
0, 4,
> +				0, &lock);
> +
> +	clk_table[SAMSUNG_DOUT_BUS] = clk_register_divider(NULL, 
"dout_bus",
> +				"dout_srp", 0, reg_base + ASS_CLK_DIV, 4, 
4, 0,
> +				&lock);
> +
> +	clk_table[SAMSUNG_DOUT_I2S] = clk_register_divider(NULL, 
"dout_i2s",
> +				"mout_i2s", 0, reg_base + ASS_CLK_DIV, 8, 
4, 0,
> +				&lock);
> +
> +	clk_table[SAMSUNG_SRP_CLK] = clk_register_gate(NULL, "srp_clk",
> +				"dout_srp", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 0, 0, &lock);
> +
> +	clk_table[SAMSUNG_I2S_BUS] = clk_register_gate(NULL, "i2s_bus",
> +				"dout_bus", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 2, 0, &lock);
> +
> +	clk_table[SAMSUNG_SCLK_I2S] = clk_register_gate(NULL, "sclk_i2s",
> +				"dout_i2s", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 3, 0, &lock);
> +
> +	clk_table[SAMSUNG_PCM_BUS] = clk_register_gate(NULL, "pcm_bus",
> +				 "sclk_pcm", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 4, 0, &lock);
> +
> +	clk_table[SAMSUNG_SCLK_PCM] = clk_register_gate(NULL, "sclk_pcm",
> +				"div_pcm0", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 5, 0, &lock);
> +
> +#ifdef CONFIG_PM_SLEEP
> +	register_syscore_ops(&samsung_audss_clk_syscore_ops);
> +#endif
> +
> +	pr_info("Exynos: Audss: clock setup completed\n");
> +}
> +CLK_OF_DECLARE(s5pv210_audss_clk, "samsung,s5pv210-audss-clock",
> +		samsung_audss_clk_init);

I'm wondering if this driver in its current state is really compatible 
with S5PV210. Quick look at documentation shows that CLK_GATE and CLK_DIV 
registers on this SoC have different layout than on Exynos SoCs.

I guess that for now the best choice would be to remove any mentions about 
S5PV210 from the patch and add them back after the driver gets proper 
support for this SoC.

> +CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
> +		samsung_audss_clk_init);
> +CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
> +		samsung_audss_clk_init);

Also if both Exynos4210 and Exynos5250 have exactly the same audss clock 
layout, there is no reason to have two compatibles for them - the 
convention is that just the first model that had this hardware is enough - 
in this case Exynos4210.

Having two different compatibles suggests that those two SoCs differ in a 
way that needs special handling, which is misleading, based on the fact 
that there is no such special handling in the driver.

Best regards,
Tomasz

[PATCH V2 1/3] clk: samsung: register audio subsystem clocks using common clock framework

[Tomasz Figa]

Hi Padmavathi,

I managed to review the patch a bit more thoroughly and I had few more 
comments. You can find them inline.

On Tuesday 07 of May 2013 12:13:34 Padmavathi Venna wrote:
> Audio subsystem is introduced in s5pv210 and exynos platforms.
> This has seperate clock controller which can control i2s0 and
> pcm0 clocks. This patch registers the audio subsystem clocks
> with the common clock framework.
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
>  drivers/clk/samsung/Makefile                       |    1 +
>  drivers/clk/samsung/clk-samsung-audss.c            |  137
> ++++++++++++++++++++ include/dt-bindings/clk/samsung-audss-clk.h       
> |   25 ++++ 4 files changed, 227 insertions(+), 0 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/clock/clk-samsung-audss.txt create
> mode 100644 drivers/clk/samsung/clk-samsung-audss.c
>  create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h
> 
> diff --git
> a/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
> b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt new
> file mode 100644
> index 0000000..ec2cd0b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
> @@ -0,0 +1,64 @@
> +* Samsung Audio Subsystem Clock Controller
> +
> +The Samsung Audio Subsystem clock controller generates and supplies
> clocks +to Audio Subsystem block available in the S5PV210 and Exynos
> SoCs. The clock +binding described here is applicable to all SoC's in
> the S5PV210 and Exynos +family.
> +
> +Required Properties:
> +
> +- compatible: should be one of the following:
> +  - "samsung,s5pv210-audss-clock" - controller compatible with all
> S5PV210 SoCs. +  - "samsung,exynos4210-audss-clock" - controller
> compatible with all Exynos4 SoCs. +  - "samsung,exynos5250-audss-clock"
> - controller compatible with all Exynos5 SoCs. +
> +- reg: physical base address and length of the controller's register
> set. +
> +- #clock-cells: should be 1.
> +
> +The following is the list of clocks generated by the controller. Each
> clock is +assigned an identifier and client nodes use this identifier
> to specify the +clock which they consume. Some of the clocks are
> available only on a particular +Exynos4 SoC and this is specified where
> applicable.
> +
> +Provided clocks:
> +
> +Clock           ID      SoC (if specific)
> +-----------------------------------------------
> +
> +mout_audss      0
> +mout_i2s        1
> +dout_srp        2
> +dout_bus        3
> +dout_i2s        4
> +srp_clk         5
> +i2s_bus         6
> +sclk_i2s        7
> +pcm_bus         8
> +sclk_pcm        9
> +
> +Example 1: An example of a clock controller node is listed below.
> +
> +clock_audss: audss-clock-controller at 03810000 {
> +	compatible = "samsung,exynos5250-audss-clock";
> +	reg = <0x03810000 0x0C>;
> +	#clock-cells = <1>;
> +};
> +
> +Example 2: I2S controller node that consumes the clock generated by the
> clock +           controller. Refer to the standard clock bindings for
> information +           about 'clocks' and 'clock-names' property.
> +
> + i2s0: i2s at 03830000 {
> +		compatible = "samsung,i2s-v5";
> +		reg = <0x03830000 0x100>;
> +		dmas = <&pdma0 10
> +			&pdma0 9
> +			&pdma0 8>;
> +		dma-names = "tx", "rx", "tx-sec";
> +		clocks = <&clock_audss SAMSUNG_I2S_BUS>,
> +			<&clock_audss SAMSUNG_I2S_BUS>,
> +			<&clock_audss SAMSUNG_SCLK_I2S>;
> +		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
> +};
> +
> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
> index b7c232e..5425fa8 100644
> --- a/drivers/clk/samsung/Makefile
> +++ b/drivers/clk/samsung/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o
>  obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4.o
>  obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
>  obj-$(CONFIG_SOC_EXYNOS5440)	+= clk-exynos5440.o
> +obj-$(CONFIG_PLAT_SAMSUNG)	+= clk-samsung-audss.o
> diff --git a/drivers/clk/samsung/clk-samsung-audss.c
> b/drivers/clk/samsung/clk-samsung-audss.c new file mode 100644
> index 0000000..97526b7
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-samsung-audss.c
> @@ -0,0 +1,137 @@
> +/*
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * Author: Padmavathi Venna <padma.v@samsung.com>
> + *
> + * 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.
> + *
> + * Common Clock Framework support for Audio Subsystem Clock Controller.
> +*/
> +
> +#include <linux/clkdev.h>
> +#include <linux/io.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/syscore_ops.h>
> +
> +#include <dt-bindings/clk/samsung-audss-clk.h>
> +
> +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
> +
> +static unsigned long reg_save[][2] = {
> +	{ASS_CLK_SRC,  0},
> +	{ASS_CLK_DIV,  0},
> +	{ASS_CLK_GATE, 0},
> +};
> +
> +/* list of all parent clock list */
> +static const char *mout_audss_p[] = { "fin_pll", "fout_epll" };
> +static const char *mout_i2s_p[] = { "mout_audss", "cdclk0",
> "sclk_audio0" }; +
> +#ifdef CONFIG_PM_SLEEP
> +static int samsung_audss_clk_suspend(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < 3; i++)
> +		reg_save[i][1] = readl(reg_base + reg_save[i][0]);
> +
> +	return 0;
> +}
> +
> +static void samsung_audss_clk_resume(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < 3; i++)
> +		writel(reg_save[i][1], reg_base + reg_save[i][0]);
> +}
> +
> +static struct syscore_ops samsung_audss_clk_syscore_ops = {
> +	.suspend	= samsung_audss_clk_suspend,
> +	.resume		= samsung_audss_clk_resume,
> +};
> +#endif /* CONFIG_PM_SLEEP */
> +
> +/* register samsung_audss clocks */
> +void __init samsung_audss_clk_init(struct device_node *np)
> +{
> +	reg_base = of_iomap(np, 0);
> +	if (!reg_base) {
> +		pr_err("%s: failed to map audss registers\n", __func__);
> +		return;
> +	}
> +
> +	clk_table = kzalloc(sizeof(struct clk *) * SAMSUNG_AUDSS_MAX_CLKS,
> +				GFP_KERNEL);
> +	if (!clk_table) {
> +		pr_err("%s: could not allocate clock lookup table\n", 
__func__);
> +		return;
> +	}
> +
> +	clk_data.clks = clk_table;
> +	clk_data.clk_num = SAMSUNG_AUDSS_MAX_CLKS;
> +	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
> +
> +	clk_table[SAMSUNG_MOUT_AUDSS] = clk_register_mux(NULL, 
"mout_audss",
> +				mout_audss_p, ARRAY_SIZE(mout_audss_p), 0,
> +				reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
> +	clk_register_clkdev(clk_table[SAMSUNG_MOUT_AUDSS], "mout_audss",
> NULL); +

Is there are a reason for this clkdev lookup to be registered?

This driver seems to be used only in DT-case, so DT-based lookup should be 
enough.

> +	clk_table[SAMSUNG_MOUT_I2S] = clk_register_mux(NULL, "mout_i2s",
> +				mout_i2s_p, ARRAY_SIZE(mout_i2s_p), 0,
> +				reg_base + ASS_CLK_SRC, 2, 2, 0, &lock);
> +	clk_register_clkdev(clk_table[SAMSUNG_MOUT_I2S], "mout_i2s", 
NULL);

Same here.

> +
> +	clk_table[SAMSUNG_DOUT_SRP] = clk_register_divider(NULL, 
"dout_srp",
> +				"mout_audss", 0, reg_base + ASS_CLK_DIV, 
0, 4,
> +				0, &lock);
> +
> +	clk_table[SAMSUNG_DOUT_BUS] = clk_register_divider(NULL, 
"dout_bus",
> +				"dout_srp", 0, reg_base + ASS_CLK_DIV, 4, 
4, 0,
> +				&lock);
> +
> +	clk_table[SAMSUNG_DOUT_I2S] = clk_register_divider(NULL, 
"dout_i2s",
> +				"mout_i2s", 0, reg_base + ASS_CLK_DIV, 8, 
4, 0,
> +				&lock);
> +
> +	clk_table[SAMSUNG_SRP_CLK] = clk_register_gate(NULL, "srp_clk",
> +				"dout_srp", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 0, 0, &lock);
> +
> +	clk_table[SAMSUNG_I2S_BUS] = clk_register_gate(NULL, "i2s_bus",
> +				"dout_bus", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 2, 0, &lock);
> +
> +	clk_table[SAMSUNG_SCLK_I2S] = clk_register_gate(NULL, "sclk_i2s",
> +				"dout_i2s", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 3, 0, &lock);
> +
> +	clk_table[SAMSUNG_PCM_BUS] = clk_register_gate(NULL, "pcm_bus",
> +				 "sclk_pcm", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 4, 0, &lock);
> +
> +	clk_table[SAMSUNG_SCLK_PCM] = clk_register_gate(NULL, "sclk_pcm",
> +				"div_pcm0", CLK_SET_RATE_PARENT,
> +				reg_base + ASS_CLK_GATE, 5, 0, &lock);
> +
> +#ifdef CONFIG_PM_SLEEP
> +	register_syscore_ops(&samsung_audss_clk_syscore_ops);
> +#endif
> +
> +	pr_info("Exynos: Audss: clock setup completed\n");
> +}
> +CLK_OF_DECLARE(s5pv210_audss_clk, "samsung,s5pv210-audss-clock",
> +		samsung_audss_clk_init);

I'm wondering if this driver in its current state is really compatible 
with S5PV210. Quick look at documentation shows that CLK_GATE and CLK_DIV 
registers on this SoC have different layout than on Exynos SoCs.

I guess that for now the best choice would be to remove any mentions about 
S5PV210 from the patch and add them back after the driver gets proper 
support for this SoC.

> +CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
> +		samsung_audss_clk_init);
> +CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
> +		samsung_audss_clk_init);

Also if both Exynos4210 and Exynos5250 have exactly the same audss clock 
layout, there is no reason to have two compatibles for them - the 
convention is that just the first model that had this hardware is enough - 
in this case Exynos4210.

Having two different compatibles suggests that those two SoCs differ in a 
way that needs special handling, which is misleading, based on the fact 
that there is no such special handling in the driver.

Best regards,
Tomasz

Re: [PATCH V2 0/3] clk: Samsung: audss: Register audio subsytem clocks using common clk framework

[Tomasz Figa]

Hi Padmavathi,

On Tuesday 07 of May 2013 12:13:33 Padmavathi Venna wrote:
> Samsung S5PV210 and Exynos SoC has a separate subsystem for audio. This
> subsystem has a internal clock controller which controls i2s0 and pcm0
> clocks. This patch series adds the Samsung audio subsytem clock to the
> common clock framework and provides the I2S controllers clock
> information in the dtsi file.
> 
> This patch series is dependent on
> https://patchwork.kernel.org/patch/2532091/
> http://www.spinics.net/lists/linux-kbuild/msg07616.html
> 
> This patch series is made based on Kukjin Kim for-next branch
> 
> Changes since V1:
> 	- Reworked on all review comments by Sylwester Nawrocki
> 	- Added a header file for all clock indexes as requested by 
Sylwester
> 	- Added different compatible names for s5pv210, exynos4 and 
exynos5
> 	- Registered the pcm clocks with common clock framework
> 
> Padmavathi Venna (3):
>   clk: samsung: register audio subsystem clocks using common clock
>     framework
>   ARM: dts: add Exynos audio subsystem clock controller node
>   ARM: dts: add clock provider information for i2s controllers in
>     Exynos5250
> 
>  .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
>  arch/arm/boot/dts/exynos5250.dtsi                  |   16 +++
>  drivers/clk/samsung/Makefile                       |    1 +
>  drivers/clk/samsung/clk-samsung-audss.c            |  137
> ++++++++++++++++++++ include/dt-bindings/clk/samsung-audss-clk.h       
> |   25 ++++ 5 files changed, 243 insertions(+), 0 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/clock/clk-samsung-audss.txt create
> mode 100644 drivers/clk/samsung/clk-samsung-audss.c
>  create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h

Also, I think that this series lacks

Cc: Mike Turquette <mturquette@linaro.org>

as he's the maintainer of the clk subsystem.

Best regards,
Tomasz

[PATCH V2 0/3] clk: Samsung: audss: Register audio subsytem clocks using common clk framework

[Tomasz Figa]

Hi Padmavathi,

On Tuesday 07 of May 2013 12:13:33 Padmavathi Venna wrote:
> Samsung S5PV210 and Exynos SoC has a separate subsystem for audio. This
> subsystem has a internal clock controller which controls i2s0 and pcm0
> clocks. This patch series adds the Samsung audio subsytem clock to the
> common clock framework and provides the I2S controllers clock
> information in the dtsi file.
> 
> This patch series is dependent on
> https://patchwork.kernel.org/patch/2532091/
> http://www.spinics.net/lists/linux-kbuild/msg07616.html
> 
> This patch series is made based on Kukjin Kim for-next branch
> 
> Changes since V1:
> 	- Reworked on all review comments by Sylwester Nawrocki
> 	- Added a header file for all clock indexes as requested by 
Sylwester
> 	- Added different compatible names for s5pv210, exynos4 and 
exynos5
> 	- Registered the pcm clocks with common clock framework
> 
> Padmavathi Venna (3):
>   clk: samsung: register audio subsystem clocks using common clock
>     framework
>   ARM: dts: add Exynos audio subsystem clock controller node
>   ARM: dts: add clock provider information for i2s controllers in
>     Exynos5250
> 
>  .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
>  arch/arm/boot/dts/exynos5250.dtsi                  |   16 +++
>  drivers/clk/samsung/Makefile                       |    1 +
>  drivers/clk/samsung/clk-samsung-audss.c            |  137
> ++++++++++++++++++++ include/dt-bindings/clk/samsung-audss-clk.h       
> |   25 ++++ 5 files changed, 243 insertions(+), 0 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/clock/clk-samsung-audss.txt create
> mode 100644 drivers/clk/samsung/clk-samsung-audss.c
>  create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h

Also, I think that this series lacks

Cc: Mike Turquette <mturquette@linaro.org>

as he's the maintainer of the clk subsystem.

Best regards,
Tomasz

Re: [PATCH V2 1/3] clk: samsung: register audio subsystem clocks using common clock framework

[Padma Venkat]

Hi Tomasz,

On Fri, May 10, 2013 at 5:51 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Padmavathi,
>
> I managed to review the patch a bit more thoroughly and I had few more
> comments. You can find them inline.

Thanks for the review.

>
> On Tuesday 07 of May 2013 12:13:34 Padmavathi Venna wrote:
>> Audio subsystem is introduced in s5pv210 and exynos platforms.
>> This has seperate clock controller which can control i2s0 and
>> pcm0 clocks. This patch registers the audio subsystem clocks
>> with the common clock framework.
>>
>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
>> ---
>>  .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
>>  drivers/clk/samsung/Makefile                       |    1 +
>>  drivers/clk/samsung/clk-samsung-audss.c            |  137
>> ++++++++++++++++++++ include/dt-bindings/clk/samsung-audss-clk.h
>> |   25 ++++ 4 files changed, 227 insertions(+), 0 deletions(-)
>>  create mode 100644
>> Documentation/devicetree/bindings/clock/clk-samsung-audss.txt create
>> mode 100644 drivers/clk/samsung/clk-samsung-audss.c
>>  create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h
>>
>> diff --git
>> a/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
>> b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt new
>> file mode 100644
>> index 0000000..ec2cd0b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
>> @@ -0,0 +1,64 @@
>> +* Samsung Audio Subsystem Clock Controller
>> +
>> +The Samsung Audio Subsystem clock controller generates and supplies
>> clocks +to Audio Subsystem block available in the S5PV210 and Exynos
>> SoCs. The clock +binding described here is applicable to all SoC's in
>> the S5PV210 and Exynos +family.
>> +
>> +Required Properties:
>> +
>> +- compatible: should be one of the following:
>> +  - "samsung,s5pv210-audss-clock" - controller compatible with all
>> S5PV210 SoCs. +  - "samsung,exynos4210-audss-clock" - controller
>> compatible with all Exynos4 SoCs. +  - "samsung,exynos5250-audss-clock"
>> - controller compatible with all Exynos5 SoCs. +
>> +- reg: physical base address and length of the controller's register
>> set. +
>> +- #clock-cells: should be 1.
>> +
>> +The following is the list of clocks generated by the controller. Each
>> clock is +assigned an identifier and client nodes use this identifier
>> to specify the +clock which they consume. Some of the clocks are
>> available only on a particular +Exynos4 SoC and this is specified where
>> applicable.
>> +
>> +Provided clocks:
>> +
>> +Clock           ID      SoC (if specific)
>> +-----------------------------------------------
>> +
>> +mout_audss      0
>> +mout_i2s        1
>> +dout_srp        2
>> +dout_bus        3
>> +dout_i2s        4
>> +srp_clk         5
>> +i2s_bus         6
>> +sclk_i2s        7
>> +pcm_bus         8
>> +sclk_pcm        9
>> +
>> +Example 1: An example of a clock controller node is listed below.
>> +
>> +clock_audss: audss-clock-controller@03810000 {
>> +     compatible = "samsung,exynos5250-audss-clock";
>> +     reg = <0x03810000 0x0C>;
>> +     #clock-cells = <1>;
>> +};
>> +
>> +Example 2: I2S controller node that consumes the clock generated by the
>> clock +           controller. Refer to the standard clock bindings for
>> information +           about 'clocks' and 'clock-names' property.
>> +
>> + i2s0: i2s@03830000 {
>> +             compatible = "samsung,i2s-v5";
>> +             reg = <0x03830000 0x100>;
>> +             dmas = <&pdma0 10
>> +                     &pdma0 9
>> +                     &pdma0 8>;
>> +             dma-names = "tx", "rx", "tx-sec";
>> +             clocks = <&clock_audss SAMSUNG_I2S_BUS>,
>> +                     <&clock_audss SAMSUNG_I2S_BUS>,
>> +                     <&clock_audss SAMSUNG_SCLK_I2S>;
>> +             clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
>> +};
>> +
>> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
>> index b7c232e..5425fa8 100644
>> --- a/drivers/clk/samsung/Makefile
>> +++ b/drivers/clk/samsung/Makefile
>> @@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK)      += clk.o clk-pll.o
>>  obj-$(CONFIG_ARCH_EXYNOS4)   += clk-exynos4.o
>>  obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o
>>  obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o
>> +obj-$(CONFIG_PLAT_SAMSUNG)   += clk-samsung-audss.o
>> diff --git a/drivers/clk/samsung/clk-samsung-audss.c
>> b/drivers/clk/samsung/clk-samsung-audss.c new file mode 100644
>> index 0000000..97526b7
>> --- /dev/null
>> +++ b/drivers/clk/samsung/clk-samsung-audss.c
>> @@ -0,0 +1,137 @@
>> +/*
>> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
>> + * Author: Padmavathi Venna <padma.v@samsung.com>
>> + *
>> + * 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.
>> + *
>> + * Common Clock Framework support for Audio Subsystem Clock Controller.
>> +*/
>> +
>> +#include <linux/clkdev.h>
>> +#include <linux/io.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/of_address.h>
>> +#include <linux/syscore_ops.h>
>> +
>> +#include <dt-bindings/clk/samsung-audss-clk.h>
>> +
>> +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
>> +
>> +static unsigned long reg_save[][2] = {
>> +     {ASS_CLK_SRC,  0},
>> +     {ASS_CLK_DIV,  0},
>> +     {ASS_CLK_GATE, 0},
>> +};
>> +
>> +/* list of all parent clock list */
>> +static const char *mout_audss_p[] = { "fin_pll", "fout_epll" };
>> +static const char *mout_i2s_p[] = { "mout_audss", "cdclk0",
>> "sclk_audio0" }; +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int samsung_audss_clk_suspend(void)
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < 3; i++)
>> +             reg_save[i][1] = readl(reg_base + reg_save[i][0]);
>> +
>> +     return 0;
>> +}
>> +
>> +static void samsung_audss_clk_resume(void)
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < 3; i++)
>> +             writel(reg_save[i][1], reg_base + reg_save[i][0]);
>> +}
>> +
>> +static struct syscore_ops samsung_audss_clk_syscore_ops = {
>> +     .suspend        = samsung_audss_clk_suspend,
>> +     .resume         = samsung_audss_clk_resume,
>> +};
>> +#endif /* CONFIG_PM_SLEEP */
>> +
>> +/* register samsung_audss clocks */
>> +void __init samsung_audss_clk_init(struct device_node *np)
>> +{
>> +     reg_base = of_iomap(np, 0);
>> +     if (!reg_base) {
>> +             pr_err("%s: failed to map audss registers\n", __func__);
>> +             return;
>> +     }
>> +
>> +     clk_table = kzalloc(sizeof(struct clk *) * SAMSUNG_AUDSS_MAX_CLKS,
>> +                             GFP_KERNEL);
>> +     if (!clk_table) {
>> +             pr_err("%s: could not allocate clock lookup table\n",
> __func__);
>> +             return;
>> +     }
>> +
>> +     clk_data.clks = clk_table;
>> +     clk_data.clk_num = SAMSUNG_AUDSS_MAX_CLKS;
>> +     of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
>> +
>> +     clk_table[SAMSUNG_MOUT_AUDSS] = clk_register_mux(NULL,
> "mout_audss",
>> +                             mout_audss_p, ARRAY_SIZE(mout_audss_p), 0,
>> +                             reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
>> +     clk_register_clkdev(clk_table[SAMSUNG_MOUT_AUDSS], "mout_audss",
>> NULL); +
>
> Is there are a reason for this clkdev lookup to be registered?
>
> This driver seems to be used only in DT-case, so DT-based lookup should be
> enough.

Ok. I will not register these clocks with clkdev. I will add as clock
entries into i2s0 node.

>
>> +     clk_table[SAMSUNG_MOUT_I2S] = clk_register_mux(NULL, "mout_i2s",
>> +                             mout_i2s_p, ARRAY_SIZE(mout_i2s_p), 0,
>> +                             reg_base + ASS_CLK_SRC, 2, 2, 0, &lock);
>> +     clk_register_clkdev(clk_table[SAMSUNG_MOUT_I2S], "mout_i2s",
> NULL);
>
> Same here.
>
>> +
>> +     clk_table[SAMSUNG_DOUT_SRP] = clk_register_divider(NULL,
> "dout_srp",
>> +                             "mout_audss", 0, reg_base + ASS_CLK_DIV,
> 0, 4,
>> +                             0, &lock);
>> +
>> +     clk_table[SAMSUNG_DOUT_BUS] = clk_register_divider(NULL,
> "dout_bus",
>> +                             "dout_srp", 0, reg_base + ASS_CLK_DIV, 4,
> 4, 0,
>> +                             &lock);
>> +
>> +     clk_table[SAMSUNG_DOUT_I2S] = clk_register_divider(NULL,
> "dout_i2s",
>> +                             "mout_i2s", 0, reg_base + ASS_CLK_DIV, 8,
> 4, 0,
>> +                             &lock);
>> +
>> +     clk_table[SAMSUNG_SRP_CLK] = clk_register_gate(NULL, "srp_clk",
>> +                             "dout_srp", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 0, 0, &lock);
>> +
>> +     clk_table[SAMSUNG_I2S_BUS] = clk_register_gate(NULL, "i2s_bus",
>> +                             "dout_bus", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 2, 0, &lock);
>> +
>> +     clk_table[SAMSUNG_SCLK_I2S] = clk_register_gate(NULL, "sclk_i2s",
>> +                             "dout_i2s", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 3, 0, &lock);
>> +
>> +     clk_table[SAMSUNG_PCM_BUS] = clk_register_gate(NULL, "pcm_bus",
>> +                              "sclk_pcm", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 4, 0, &lock);
>> +
>> +     clk_table[SAMSUNG_SCLK_PCM] = clk_register_gate(NULL, "sclk_pcm",
>> +                             "div_pcm0", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 5, 0, &lock);
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +     register_syscore_ops(&samsung_audss_clk_syscore_ops);
>> +#endif
>> +
>> +     pr_info("Exynos: Audss: clock setup completed\n");
>> +}
>> +CLK_OF_DECLARE(s5pv210_audss_clk, "samsung,s5pv210-audss-clock",
>> +             samsung_audss_clk_init);
>
> I'm wondering if this driver in its current state is really compatible
> with S5PV210. Quick look at documentation shows that CLK_GATE and CLK_DIV
> registers on this SoC have different layout than on Exynos SoCs.

Yes. There are some differences. As of now I will remove v210.

>
> I guess that for now the best choice would be to remove any mentions about
> S5PV210 from the patch and add them back after the driver gets proper
> support for this SoC.
>
>> +CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
>> +             samsung_audss_clk_init);
>> +CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
>> +             samsung_audss_clk_init);
>
> Also if both Exynos4210 and Exynos5250 have exactly the same audss clock
> layout, there is no reason to have two compatibles for them - the
> convention is that just the first model that had this hardware is enough -
> in this case Exynos4210.
>
> Having two different compatibles suggests that those two SoCs differ in a
> way that needs special handling, which is misleading, based on the fact
> that there is no such special handling in the driver.

There is only one difference between Exynos4 and Exynos5 is bit 1 of
CLK_GATE register where in Exynos5 it is reserved and Exynos4 it is
gate to IntMEM. I am not sure if we use this bit some where? So is it
okey to have same compatible with this diff?

I will post next version of patches after the dependencies are merged
into mainline(dtc+cpp patches by Stephen).

>
> Best regards,
> Tomasz
>
Thanks
Padma

[PATCH V2 1/3] clk: samsung: register audio subsystem clocks using common clock framework

[Padma Venkat]

Hi Tomasz,

On Fri, May 10, 2013 at 5:51 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Padmavathi,
>
> I managed to review the patch a bit more thoroughly and I had few more
> comments. You can find them inline.

Thanks for the review.

>
> On Tuesday 07 of May 2013 12:13:34 Padmavathi Venna wrote:
>> Audio subsystem is introduced in s5pv210 and exynos platforms.
>> This has seperate clock controller which can control i2s0 and
>> pcm0 clocks. This patch registers the audio subsystem clocks
>> with the common clock framework.
>>
>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
>> ---
>>  .../bindings/clock/clk-samsung-audss.txt           |   64 +++++++++
>>  drivers/clk/samsung/Makefile                       |    1 +
>>  drivers/clk/samsung/clk-samsung-audss.c            |  137
>> ++++++++++++++++++++ include/dt-bindings/clk/samsung-audss-clk.h
>> |   25 ++++ 4 files changed, 227 insertions(+), 0 deletions(-)
>>  create mode 100644
>> Documentation/devicetree/bindings/clock/clk-samsung-audss.txt create
>> mode 100644 drivers/clk/samsung/clk-samsung-audss.c
>>  create mode 100644 include/dt-bindings/clk/samsung-audss-clk.h
>>
>> diff --git
>> a/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
>> b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt new
>> file mode 100644
>> index 0000000..ec2cd0b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/clk-samsung-audss.txt
>> @@ -0,0 +1,64 @@
>> +* Samsung Audio Subsystem Clock Controller
>> +
>> +The Samsung Audio Subsystem clock controller generates and supplies
>> clocks +to Audio Subsystem block available in the S5PV210 and Exynos
>> SoCs. The clock +binding described here is applicable to all SoC's in
>> the S5PV210 and Exynos +family.
>> +
>> +Required Properties:
>> +
>> +- compatible: should be one of the following:
>> +  - "samsung,s5pv210-audss-clock" - controller compatible with all
>> S5PV210 SoCs. +  - "samsung,exynos4210-audss-clock" - controller
>> compatible with all Exynos4 SoCs. +  - "samsung,exynos5250-audss-clock"
>> - controller compatible with all Exynos5 SoCs. +
>> +- reg: physical base address and length of the controller's register
>> set. +
>> +- #clock-cells: should be 1.
>> +
>> +The following is the list of clocks generated by the controller. Each
>> clock is +assigned an identifier and client nodes use this identifier
>> to specify the +clock which they consume. Some of the clocks are
>> available only on a particular +Exynos4 SoC and this is specified where
>> applicable.
>> +
>> +Provided clocks:
>> +
>> +Clock           ID      SoC (if specific)
>> +-----------------------------------------------
>> +
>> +mout_audss      0
>> +mout_i2s        1
>> +dout_srp        2
>> +dout_bus        3
>> +dout_i2s        4
>> +srp_clk         5
>> +i2s_bus         6
>> +sclk_i2s        7
>> +pcm_bus         8
>> +sclk_pcm        9
>> +
>> +Example 1: An example of a clock controller node is listed below.
>> +
>> +clock_audss: audss-clock-controller at 03810000 {
>> +     compatible = "samsung,exynos5250-audss-clock";
>> +     reg = <0x03810000 0x0C>;
>> +     #clock-cells = <1>;
>> +};
>> +
>> +Example 2: I2S controller node that consumes the clock generated by the
>> clock +           controller. Refer to the standard clock bindings for
>> information +           about 'clocks' and 'clock-names' property.
>> +
>> + i2s0: i2s at 03830000 {
>> +             compatible = "samsung,i2s-v5";
>> +             reg = <0x03830000 0x100>;
>> +             dmas = <&pdma0 10
>> +                     &pdma0 9
>> +                     &pdma0 8>;
>> +             dma-names = "tx", "rx", "tx-sec";
>> +             clocks = <&clock_audss SAMSUNG_I2S_BUS>,
>> +                     <&clock_audss SAMSUNG_I2S_BUS>,
>> +                     <&clock_audss SAMSUNG_SCLK_I2S>;
>> +             clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
>> +};
>> +
>> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
>> index b7c232e..5425fa8 100644
>> --- a/drivers/clk/samsung/Makefile
>> +++ b/drivers/clk/samsung/Makefile
>> @@ -6,3 +6,4 @@ obj-$(CONFIG_COMMON_CLK)      += clk.o clk-pll.o
>>  obj-$(CONFIG_ARCH_EXYNOS4)   += clk-exynos4.o
>>  obj-$(CONFIG_SOC_EXYNOS5250) += clk-exynos5250.o
>>  obj-$(CONFIG_SOC_EXYNOS5440) += clk-exynos5440.o
>> +obj-$(CONFIG_PLAT_SAMSUNG)   += clk-samsung-audss.o
>> diff --git a/drivers/clk/samsung/clk-samsung-audss.c
>> b/drivers/clk/samsung/clk-samsung-audss.c new file mode 100644
>> index 0000000..97526b7
>> --- /dev/null
>> +++ b/drivers/clk/samsung/clk-samsung-audss.c
>> @@ -0,0 +1,137 @@
>> +/*
>> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
>> + * Author: Padmavathi Venna <padma.v@samsung.com>
>> + *
>> + * 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.
>> + *
>> + * Common Clock Framework support for Audio Subsystem Clock Controller.
>> +*/
>> +
>> +#include <linux/clkdev.h>
>> +#include <linux/io.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/of_address.h>
>> +#include <linux/syscore_ops.h>
>> +
>> +#include <dt-bindings/clk/samsung-audss-clk.h>
>> +
>> +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
>> +
>> +static unsigned long reg_save[][2] = {
>> +     {ASS_CLK_SRC,  0},
>> +     {ASS_CLK_DIV,  0},
>> +     {ASS_CLK_GATE, 0},
>> +};
>> +
>> +/* list of all parent clock list */
>> +static const char *mout_audss_p[] = { "fin_pll", "fout_epll" };
>> +static const char *mout_i2s_p[] = { "mout_audss", "cdclk0",
>> "sclk_audio0" }; +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int samsung_audss_clk_suspend(void)
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < 3; i++)
>> +             reg_save[i][1] = readl(reg_base + reg_save[i][0]);
>> +
>> +     return 0;
>> +}
>> +
>> +static void samsung_audss_clk_resume(void)
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < 3; i++)
>> +             writel(reg_save[i][1], reg_base + reg_save[i][0]);
>> +}
>> +
>> +static struct syscore_ops samsung_audss_clk_syscore_ops = {
>> +     .suspend        = samsung_audss_clk_suspend,
>> +     .resume         = samsung_audss_clk_resume,
>> +};
>> +#endif /* CONFIG_PM_SLEEP */
>> +
>> +/* register samsung_audss clocks */
>> +void __init samsung_audss_clk_init(struct device_node *np)
>> +{
>> +     reg_base = of_iomap(np, 0);
>> +     if (!reg_base) {
>> +             pr_err("%s: failed to map audss registers\n", __func__);
>> +             return;
>> +     }
>> +
>> +     clk_table = kzalloc(sizeof(struct clk *) * SAMSUNG_AUDSS_MAX_CLKS,
>> +                             GFP_KERNEL);
>> +     if (!clk_table) {
>> +             pr_err("%s: could not allocate clock lookup table\n",
> __func__);
>> +             return;
>> +     }
>> +
>> +     clk_data.clks = clk_table;
>> +     clk_data.clk_num = SAMSUNG_AUDSS_MAX_CLKS;
>> +     of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
>> +
>> +     clk_table[SAMSUNG_MOUT_AUDSS] = clk_register_mux(NULL,
> "mout_audss",
>> +                             mout_audss_p, ARRAY_SIZE(mout_audss_p), 0,
>> +                             reg_base + ASS_CLK_SRC, 0, 1, 0, &lock);
>> +     clk_register_clkdev(clk_table[SAMSUNG_MOUT_AUDSS], "mout_audss",
>> NULL); +
>
> Is there are a reason for this clkdev lookup to be registered?
>
> This driver seems to be used only in DT-case, so DT-based lookup should be
> enough.

Ok. I will not register these clocks with clkdev. I will add as clock
entries into i2s0 node.

>
>> +     clk_table[SAMSUNG_MOUT_I2S] = clk_register_mux(NULL, "mout_i2s",
>> +                             mout_i2s_p, ARRAY_SIZE(mout_i2s_p), 0,
>> +                             reg_base + ASS_CLK_SRC, 2, 2, 0, &lock);
>> +     clk_register_clkdev(clk_table[SAMSUNG_MOUT_I2S], "mout_i2s",
> NULL);
>
> Same here.
>
>> +
>> +     clk_table[SAMSUNG_DOUT_SRP] = clk_register_divider(NULL,
> "dout_srp",
>> +                             "mout_audss", 0, reg_base + ASS_CLK_DIV,
> 0, 4,
>> +                             0, &lock);
>> +
>> +     clk_table[SAMSUNG_DOUT_BUS] = clk_register_divider(NULL,
> "dout_bus",
>> +                             "dout_srp", 0, reg_base + ASS_CLK_DIV, 4,
> 4, 0,
>> +                             &lock);
>> +
>> +     clk_table[SAMSUNG_DOUT_I2S] = clk_register_divider(NULL,
> "dout_i2s",
>> +                             "mout_i2s", 0, reg_base + ASS_CLK_DIV, 8,
> 4, 0,
>> +                             &lock);
>> +
>> +     clk_table[SAMSUNG_SRP_CLK] = clk_register_gate(NULL, "srp_clk",
>> +                             "dout_srp", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 0, 0, &lock);
>> +
>> +     clk_table[SAMSUNG_I2S_BUS] = clk_register_gate(NULL, "i2s_bus",
>> +                             "dout_bus", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 2, 0, &lock);
>> +
>> +     clk_table[SAMSUNG_SCLK_I2S] = clk_register_gate(NULL, "sclk_i2s",
>> +                             "dout_i2s", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 3, 0, &lock);
>> +
>> +     clk_table[SAMSUNG_PCM_BUS] = clk_register_gate(NULL, "pcm_bus",
>> +                              "sclk_pcm", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 4, 0, &lock);
>> +
>> +     clk_table[SAMSUNG_SCLK_PCM] = clk_register_gate(NULL, "sclk_pcm",
>> +                             "div_pcm0", CLK_SET_RATE_PARENT,
>> +                             reg_base + ASS_CLK_GATE, 5, 0, &lock);
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +     register_syscore_ops(&samsung_audss_clk_syscore_ops);
>> +#endif
>> +
>> +     pr_info("Exynos: Audss: clock setup completed\n");
>> +}
>> +CLK_OF_DECLARE(s5pv210_audss_clk, "samsung,s5pv210-audss-clock",
>> +             samsung_audss_clk_init);
>
> I'm wondering if this driver in its current state is really compatible
> with S5PV210. Quick look at documentation shows that CLK_GATE and CLK_DIV
> registers on this SoC have different layout than on Exynos SoCs.

Yes. There are some differences. As of now I will remove v210.

>
> I guess that for now the best choice would be to remove any mentions about
> S5PV210 from the patch and add them back after the driver gets proper
> support for this SoC.
>
>> +CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
>> +             samsung_audss_clk_init);
>> +CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
>> +             samsung_audss_clk_init);
>
> Also if both Exynos4210 and Exynos5250 have exactly the same audss clock
> layout, there is no reason to have two compatibles for them - the
> convention is that just the first model that had this hardware is enough -
> in this case Exynos4210.
>
> Having two different compatibles suggests that those two SoCs differ in a
> way that needs special handling, which is misleading, based on the fact
> that there is no such special handling in the driver.

There is only one difference between Exynos4 and Exynos5 is bit 1 of
CLK_GATE register where in Exynos5 it is reserved and Exynos4 it is
gate to IntMEM. I am not sure if we use this bit some where? So is it
okey to have same compatible with this diff?

I will post next version of patches after the dependencies are merged
into mainline(dtc+cpp patches by Stephen).

>
> Best regards,
> Tomasz
>
Thanks
Padma

Re: [PATCH V2 1/3] clk: samsung: register audio subsystem clocks using common clock framework

[Sylwester Nawrocki]

Hi,

On 05/11/2013 12:13 PM, Padma Venkat wrote:
>>> +CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
>>> >>  +             samsung_audss_clk_init);
>>> >>  +CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
>>> >>  +             samsung_audss_clk_init);
>> >
>> >  Also if both Exynos4210 and Exynos5250 have exactly the same audss clock
>> >  layout, there is no reason to have two compatibles for them - the
>> >  convention is that just the first model that had this hardware is enough -
>> >  in this case Exynos4210.
>> >
>> >  Having two different compatibles suggests that those two SoCs differ in a
>> >  way that needs special handling, which is misleading, based on the fact
>> >  that there is no such special handling in the driver.
>
> There is only one difference between Exynos4 and Exynos5 is bit 1 of
> CLK_GATE register where in Exynos5 it is reserved and Exynos4 it is
> gate to IntMEM. I am not sure if we use this bit some where? So is it
> okey to have same compatible with this diff?

I think such difference warrants separate compatible properties, as 
Exynos5250
seems to be not compatible with Exynos4210 in that case. Reserved bits 
should
be left untouched.

I wouldn't be surprised to see more differences we might be overlooking 
now.
IMHO it's better to be save than sorry, keeping both 'compatible' 
strings as
they are now.

Regards,
Sylwester

[PATCH V2 1/3] clk: samsung: register audio subsystem clocks using common clock framework

[Sylwester Nawrocki]

Hi,

On 05/11/2013 12:13 PM, Padma Venkat wrote:
>>> +CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
>>> >>  +             samsung_audss_clk_init);
>>> >>  +CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
>>> >>  +             samsung_audss_clk_init);
>> >
>> >  Also if both Exynos4210 and Exynos5250 have exactly the same audss clock
>> >  layout, there is no reason to have two compatibles for them - the
>> >  convention is that just the first model that had this hardware is enough -
>> >  in this case Exynos4210.
>> >
>> >  Having two different compatibles suggests that those two SoCs differ in a
>> >  way that needs special handling, which is misleading, based on the fact
>> >  that there is no such special handling in the driver.
>
> There is only one difference between Exynos4 and Exynos5 is bit 1 of
> CLK_GATE register where in Exynos5 it is reserved and Exynos4 it is
> gate to IntMEM. I am not sure if we use this bit some where? So is it
> okey to have same compatible with this diff?

I think such difference warrants separate compatible properties, as 
Exynos5250
seems to be not compatible with Exynos4210 in that case. Reserved bits 
should
be left untouched.

I wouldn't be surprised to see more differences we might be overlooking 
now.
IMHO it's better to be save than sorry, keeping both 'compatible' 
strings as
they are now.

Regards,
Sylwester