Skip to contentSkip to conversation

clk: exynos-audss: convert to platform device

PATCH Superseded Andrew Bresticker 11 participants 49 messages Sep 20, 2013 – Jan 4, 2014 1 message not in the archive

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/6 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.

---
 drivers/clk/samsung/clk-exynos-audss.c | 71 +++++++++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 13 deletions(-)
drivers/clk/samsung/clk-exynos-audss.c +58 -13
index 39b40aa..7571e88 100644
@@ -14,6 +14,8 @@
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include <dt-bindings/clk/exynos-audss-clk.h>
 
@@ -62,24 +64,29 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 #endif /* CONFIG_PM_SLEEP */
 
 /* register exynos_audss clocks */
-static void __init exynos_audss_clk_init(struct device_node *np)
+static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
-	reg_base = of_iomap(np, 0);
-	if (!reg_base) {
-		pr_err("%s: failed to map audss registers\n", __func__);
-		return;
+	struct resource *res;
+
+	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 = kzalloc(sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
+	clk_table = devm_kzalloc(&pdev->dev,
+				sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
 				GFP_KERNEL);
 	if (!clk_table) {
-		pr_err("%s: could not allocate clk lookup table\n", __func__);
-		return;
+		dev_err(&pdev->dev, "could not allocate clk lookup table\n");
+		return -ENOMEM;
 	}
 
 	clk_data.clks = clk_table;
 	clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
-	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+	of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
+				&clk_data);
 
 	clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
 				mout_audss_p, ARRAY_SIZE(mout_audss_p),
@@ -128,8 +135,46 @@ static void __init exynos_audss_clk_init(struct device_node *np)
 #endif
 
 	pr_info("Exynos: Audss: clock setup completed\n");
+
+	return 0;
+}
+
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+	of_clk_del_provider(pdev->dev.of_node);
+
+	return 0;
 }
-CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
-		exynos_audss_clk_init);
-CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
-		exynos_audss_clk_init);
+
+static const struct of_device_id exynos_audss_clk_of_match[] = {
+	{ .compatible = "samsung,exynos4210-audss-clock", },
+	{ .compatible = "samsung,exynos5250-audss-clock", },
+	{},
+};
+
+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)
+{
+	return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);
+
+static void __init exynos_audss_clk_exit(void)
+{
+	platform_driver_unregister(&exynos_audss_clk_driver);
+}
+module_exit(exynos_audss_clk_exit);
+
+MODULE_AUTHOR("Padmavathi Venna <[email protected]>");
+MODULE_DESCRIPTION("Exynos AudioSS Clock Controller");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:exynos-audss-clk");
Signature · 1 line
1.8.4
#1 lore raw

PATCH 2/6 clk: exynos-audss: allow input clocks to be specified in device tree

This allows the input clocks to the Exynos AudioSS block to be specified via device-tree bindings. Default names will be used when an input clock is not given.

---
 .../devicetree/bindings/clock/clk-exynos-audss.txt | 33 ++++++++++++++++++++--
 drivers/clk/samsung/clk-exynos-audss.c             | 25 ++++++++++++----
 2 files changed, 51 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +31 -2
index 75e2e19..d51a2f9 100644
@@ -14,6 +14,23 @@ Required Properties:
 
 - #clock-cells: should be 1.
 
+Optional Properties:
+
+- clocks:
+  - pll_ref: Fixed rate PLL reference clock, parent of mout_audss. "fin_pll"
+    is used if not specified.
+  - pll_in: Input PLL to the AudioSS block, parent of mout_audss. "fout_epll"
+    is used if not specified.
+  - cdclk: External i2s clock, parent of mout_i2s. "cdclk0" is used if not
+    specified.
+  - sclk_audio: Audio bus clock, parent of mout_i2s. "sclk_audio0" is used if
+    not specified.
+  - sclk_pcm_in: PCM clock, parent of sclk_pcm.  "sclk_pcm0" is used if not
+    specified.
+
+- clock-names: Aliases for the above clocks. They should be "pll_ref",
+  "pll_in", "cdclk", "sclk_audio", and "sclk_pcm_in" respectively.
+
 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
@@ -35,15 +52,27 @@ sclk_i2s        7
 pcm_bus         8
 sclk_pcm        9
 
-Example 1: An example of a clock controller node is listed below.
+Example 1: An example of a clock controller node using the default input
+	   clock names is listed below.
+
+clock_audss: audss-clock-controller at 3810000 {
+	compatible = "samsung,exynos5250-audss-clock";
+	reg = <0x03810000 0x0C>;
+	#clock-cells = <1>;
+};
+
+Example 2: An example of a clock controller node with audio bus input clock
+	   specified is listed below.
 
 clock_audss: audss-clock-controller at 3810000 {
 	compatible = "samsung,exynos5250-audss-clock";
 	reg = <0x03810000 0x0C>;
 	#clock-cells = <1>;
+	clocks = <&clock 138>;
+	clock-names = "sclk_audio";
 };
 
-Example 2: I2S controller node that consumes the clock generated by the clock
+Example 3: 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.
 
drivers/clk/samsung/clk-exynos-audss.c +20 -5
index 7571e88..aac5342 100644
@@ -34,10 +34,6 @@ static unsigned long reg_save[][2] = {
 	{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 exynos_audss_clk_suspend(void)
 {
@@ -66,6 +62,10 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 /* register exynos_audss clocks */
 static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
+	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;
 	struct resource *res;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -88,11 +88,23 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
 				&clk_data);
 
+	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);
 	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,
@@ -126,8 +138,11 @@ 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",
-				"div_pcm0", CLK_SET_RATE_PARENT,
+				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
 #ifdef CONFIG_PM_SLEEP
Signature · 1 line
1.8.4
#2 lore raw

Re: [PATCH 2/6] clk: exynos-audss: allow input clocks to be specified in device tree

Hi Andrew,

Quoted 3 lines · On Friday 20 of September 2013 14:13:53 Andrew Bresticker wrote:

This allows the input clocks to the Exynos AudioSS block to be specified via device-tree bindings. Default names will be used when an input clock is not given.

[snip]

Quoted 13 lines
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +4 -0
+Optional Properties:
+
+- clocks:
+  - pll_ref: Fixed rate PLL reference clock, parent of mout_audss.

"fin_pll" + is used if not specified.
+ - pll_in: Input PLL to the AudioSS block, parent of mout_audss.
"fout_epll" + is used if not specified.
+ - cdclk: External i2s clock, parent of mout_i2s. "cdclk0" is used if
not + specified.
+ - sclk_audio: Audio bus clock, parent of mout_i2s. "sclk_audio0" is
used if + not specified.
+ - sclk_pcm_in: PCM clock, parent of sclk_pcm. "sclk_pcm0" is used if
not + specified.

Documentation/devicetree/bindings/clock/clk-exynos-audss.txt Required Properties: · line 32

Quoted 3 lines
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +3 -0
+
+- clock-names: Aliases for the above clocks. They should be "pll_ref",
+  "pll_in", "cdclk", "sclk_audio", and "sclk_pcm_in" respectively.

I'd say that it should be preferred to specify all the input clocks appropriate for particular SoCs to fully represent the hardware. So the properties above should be marked as required (depending on SoC probably).

The fallback to default names in the driver should stay, though, to support older device trees.

Best regards,
Tomasz

#3 lore raw

PATCH 3/6 clk: exynos5250: add clock ID for div_pcm0

In reply to Andrew Bresticker

There is no gate for the PCM clock input to the AudioSS block, so the parent of sclk_pcm is div_pcm0. Add a clock ID for it so that we can reference it in device trees.

---
 Documentation/devicetree/bindings/clock/exynos5250-clock.txt | 1 +
 drivers/clk/samsung/clk-exynos5250.c                         | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/clock/exynos5250-clock.txt +1 -0
index 24765c1..67e9a47 100644
@@ -62,6 +62,7 @@ clock which they consume.
   div_i2s1		157
   div_i2s2		158
   sclk_hdmiphy		159
+  div_pcm0		160
 
 
    [Peripheral Clock Gates]
drivers/clk/samsung/clk-exynos5250.c +2 -2
index adf3234..dec5376 100644
@@ -108,7 +108,7 @@ enum exynos5250_clks {
 	sclk_mmc0, sclk_mmc1, sclk_mmc2, sclk_mmc3, sclk_sata, sclk_usb3,
 	sclk_jpeg, sclk_uart0, sclk_uart1, sclk_uart2, sclk_uart3, sclk_pwm,
 	sclk_audio1, sclk_audio2, sclk_spdif, sclk_spi0, sclk_spi1, sclk_spi2,
-	div_i2s1, div_i2s2, sclk_hdmiphy,
+	div_i2s1, div_i2s2, sclk_hdmiphy, div_pcm0,
 
 	/* gate clocks */
 	gscl0 = 256, gscl1, gscl2, gscl3, gscl_wa, gscl_wb, smmu_gscl0,
@@ -301,7 +301,7 @@ static struct samsung_div_clock exynos5250_div_clks[] __initdata = {
 	DIV(none, "div_dp", "mout_dp", DIV_DISP1_0, 24, 4),
 	DIV(none, "div_jpeg", "mout_jpeg", DIV_GEN, 4, 4),
 	DIV(none, "div_audio0", "mout_audio0", DIV_MAU, 0, 4),
-	DIV(none, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
+	DIV(div_pcm0, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
 	DIV(none, "div_sata", "mout_sata", DIV_FSYS0, 20, 4),
 	DIV(none, "div_usb3", "mout_usb3", DIV_FSYS0, 24, 4),
 	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
Signature · 1 line
1.8.4
Reviewed-by on v1 3/6
#4 lore raw

Re: [PATCH 3/6] clk: exynos5250: add clock ID for div_pcm0

Hi Andrew,

Quoted 9 lines · On Friday 20 of September 2013 14:13:54 Andrew Bresticker wrote:

There is no gate for the PCM clock input to the AudioSS block, so the parent of sclk_pcm is div_pcm0. Add a clock ID for it so that we can reference it in device trees.

---
 Documentation/devicetree/bindings/clock/exynos5250-clock.txt | 1 +
 drivers/clk/samsung/clk-exynos5250.c                         | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

Best regards,
Tomasz

#5 lore raw

PATCH 4/6 ARM: dts: exynos5250: add sclk_pcm_in to audss clock controller

In reply to Andrew Bresticker

The parent of sclk_pcm in the AudioSS block is div_pcm0.

---
 arch/arm/boot/dts/exynos5250.dtsi | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/boot/dts/exynos5250.dtsi +2 -0
index 7d7cc77..bedc7a8 100644
@@ -88,6 +88,8 @@
 		compatible = "samsung,exynos5250-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
+		clocks = <&clock 160>;
+		clock-names = "sclk_pcm_in";
 	};
 
 	timer {
Signature · 1 line
1.8.4
#6 lore raw

Re: [PATCH 4/6] ARM: dts: exynos5250: add sclk_pcm_in to audss clock controller

Hi Andrew,

Quoted 12 lines · On Friday 20 of September 2013 14:13:55 Andrew Bresticker wrote:

The parent of sclk_pcm in the AudioSS block is div_pcm0.

---
 arch/arm/boot/dts/exynos5250.dtsi | 2 ++
 1 file changed, 2 insertions(+)
(quoted diff) +0 -0

b/arch/arm/boot/dts/exynos5250.dtsi index 7d7cc77..bedc7a8 100644

arch/arm/boot/dts/exynos5250.dtsi line 92

Quoted 5 lines
arch/arm/boot/dts/exynos5250.dtsi +2 -0
@@ -88,6 +88,8 @@
 		compatible = "samsung,exynos5250-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
+		clocks = <&clock 160>;
+		clock-names = "sclk_pcm_in";

IMHO it would be reasonable to explicitly specify all the input clocks here.

Best regards,
Tomasz

#7 lore raw

PATCH 5/6 clk: exynos-audss: add support for Exynos 5420

In reply to Andrew Bresticker

The AudioSS block on Exynos 5420 has an additional clock gate for the ADMA bus clock.

---
 Documentation/devicetree/bindings/clock/clk-exynos-audss.txt | 7 +++++--
 drivers/clk/samsung/clk-exynos-audss.c                       | 8 ++++++++
 include/dt-bindings/clk/exynos-audss-clk.h                   | 3 ++-
 3 files changed, 15 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +5 -2
index d51a2f9..a10c648 100644
@@ -8,8 +8,10 @@ Required Properties:
 
 - compatible: should be one of the following:
   - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.
-  - "samsung,exynos5250-audss-clock" - controller compatible with all Exynos5 SoCs.
-
+  - "samsung,exynos5250-audss-clock" - controller compatible with Exynos5250
+    SoCs.
+  - "samsung,exynos5420-audss-clock" - controller compatible with Exynos5420
+    SoCs.
 - reg: physical base address and length of the controller's register set.
 
 - #clock-cells: should be 1.
@@ -51,6 +53,7 @@ i2s_bus         6
 sclk_i2s        7
 pcm_bus         8
 sclk_pcm        9
+adma            10      Exynos5420
 
 Example 1: An example of a clock controller node using the default input
 	   clock names is listed below.
drivers/clk/samsung/clk-exynos-audss.c +8 -0
index aac5342..07c8dbd 100644
@@ -145,6 +145,13 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
+	if (of_device_is_compatible(pdev->dev.of_node,
+					"samsung,exynos5420-audss-clock")) {
+		clk_table[EXYNOS_ADMA] = clk_register_gate(NULL, "adma",
+				"dout_srp", CLK_SET_RATE_PARENT,
+				reg_base + ASS_CLK_GATE, 9, 0, &lock);
+	}
+
 #ifdef CONFIG_PM_SLEEP
 	register_syscore_ops(&exynos_audss_clk_syscore_ops);
 #endif
@@ -164,6 +171,7 @@ static int exynos_audss_clk_remove(struct platform_device *pdev)
 static const struct of_device_id exynos_audss_clk_of_match[] = {
 	{ .compatible = "samsung,exynos4210-audss-clock", },
 	{ .compatible = "samsung,exynos5250-audss-clock", },
+	{ .compatible = "samsung,exynos5420-audss-clock", },
 	{},
 };
 
include/dt-bindings/clk/exynos-audss-clk.h +2 -1
index 8279f42..0ae6f5a 100644
@@ -19,7 +19,8 @@
 #define EXYNOS_SCLK_I2S	7
 #define EXYNOS_PCM_BUS		8
 #define EXYNOS_SCLK_PCM	9
+#define EXYNOS_ADMA		10
 
-#define EXYNOS_AUDSS_MAX_CLKS	10
+#define EXYNOS_AUDSS_MAX_CLKS	11
 
 #endif
Signature · 1 line
1.8.4
#8 lore raw

Re: [PATCH 5/6] clk: exynos-audss: add support for Exynos 5420

Hi Andrew,

Quoted 9 lines · On Friday 20 of September 2013 14:13:56 Andrew Bresticker wrote:

The AudioSS block on Exynos 5420 has an additional clock gate for the ADMA bus clock.

---
 Documentation/devicetree/bindings/clock/clk-exynos-audss.txt | 7

+++++-- drivers/clk/samsung/clk-exynos-audss.c |
8 ++++++++ include/dt-bindings/clk/exynos-audss-clk.h | 3 ++- 3 files changed, 15 insertions(+), 3 deletions(-)

[snip]

Review

Quoted 2 lines
(quoted diff) +2 -0
+	if (of_device_is_compatible(pdev->dev.of_node,
+					"samsung,exynos5420-audss-clock")) 

{

I don't really like using such string based conditions in the code, but I guess it's just a matter of preference.

I'd introduce an enum to represent supported variants and put them into the OF match table or maybe even a struct that would have a bool named has_adma_clock, true for Exynos5420 variant.

Best regards,
Tomasz

#9 lore raw

PATCH 6/6 ARM: dts: exynos5420: add sclk_pcm_in to audss clock controller

In reply to Andrew Bresticker

The parent of sclk_pcm in the AudioSS block is sclk_maupcm0.

---
 arch/arm/boot/dts/exynos5420.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/exynos5420.dtsi +2 -2
index d537cd7..339ab93 100644
@@ -72,8 +72,8 @@
 		compatible = "samsung,exynos5420-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
-		clocks = <&clock 148>;
-		clock-names = "sclk_audio";
+		clocks = <&clock 148>, <&clock 149>;
+		clock-names = "sclk_audio", "sclk_pcm_in";
 	};
 
 	codec at 11000000 {
Signature · 1 line
1.8.4
#10 lore raw

Re: [PATCH 6/6] ARM: dts: exynos5420: add sclk_pcm_in to audss clock controller

Hi Andrew,

Quoted 12 lines · On Friday 20 of September 2013 14:13:57 Andrew Bresticker wrote:

The parent of sclk_pcm in the AudioSS block is sclk_maupcm0.

---
 arch/arm/boot/dts/exynos5420.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
(quoted diff) +0 -0

b/arch/arm/boot/dts/exynos5420.dtsi index d537cd7..339ab93 100644

arch/arm/boot/dts/exynos5420.dtsi line 76

Quoted 7 lines
arch/arm/boot/dts/exynos5420.dtsi +2 -2
@@ -72,8 +72,8 @@
 		compatible = "samsung,exynos5420-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
-		clocks = <&clock 148>;
-		clock-names = "sclk_audio";
+		clocks = <&clock 148>, <&clock 149>;
+		clock-names = "sclk_audio", "sclk_pcm_in";

Same note as for Exynos 5250. I think it would be more reasonable to specify all the input clocks here.

Best regards,
Tomasz

#11 lore raw
In reply to Andrew Bresticker

Hi Andrew,

This patch looks good overall, but I have some minor comments inline.

Quoted 14 lines · On Friday 20 of September 2013 14:13:52 Andrew Bresticker wrote:

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.

---
 drivers/clk/samsung/clk-exynos-audss.c | 71

+++++++++++++++++++++++++++------- 1 file changed, 58 insertions(+), 13 deletions(-)

[snip]

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 147

Quoted 6 lines
drivers/clk/samsung/clk-exynos-audss.c +5 -0
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+	of_clk_del_provider(pdev->dev.of_node);
+
+	return 0;
 }

Don't we need to unregister all the registered clocks in remove? This also leads to another question: Do we even need removal support for this driver?

Quoted 16 lines
drivers/clk/samsung/clk-exynos-audss.c +12 -4
-CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
-		exynos_audss_clk_init);
-CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
-		exynos_audss_clk_init);
+
+static const struct of_device_id exynos_audss_clk_of_match[] = {
+	{ .compatible = "samsung,exynos4210-audss-clock", },
+	{ .compatible = "samsung,exynos5250-audss-clock", },
+	{},
+};
+
+static struct platform_driver exynos_audss_clk_driver = {
+	.driver	= {
+		.name = "exynos-audss-clk",
+		.owner = THIS_MODULE,
+		.of_match_table = exynos_audss_clk_of_match,

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 169

Quoted 10 lines
drivers/clk/samsung/clk-exynos-audss.c +10 -0
+	},
+	.probe = exynos_audss_clk_probe,
+	.remove = exynos_audss_clk_remove,
+};
+
+static int __init exynos_audss_clk_init(void)
+{
+	return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);

Does it need to be core_initcall? Drivers depending on clocks provided by this driver should be able to defer probing if they are probed before this driver.

Then you would be able to simply use module_platform_driver() below.

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 178

Quoted 8 lines
drivers/clk/samsung/clk-exynos-audss.c +8 -0
+static void __init exynos_audss_clk_exit(void)
+{
+	platform_driver_unregister(&exynos_audss_clk_driver);
+}
+module_exit(exynos_audss_clk_exit);
+
+MODULE_AUTHOR("Padmavathi Venna <[email protected]>");
+MODULE_DESCRIPTION("Exynos AudioSS Clock Controller");

nit: IMHO Audio Subsystem instead of AudioSS would be more meaningful.

Quoted 1 line

+MODULE_LICENSE("GPL");

This should be GPL v2.

Best regards,
Tomasz

#12 lore raw

PATCH 1/6 clk: exynos-audss: convert to platform device

Hi Tomasz,

Quoted 10 lines

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 147

Quoted 6 lines
drivers/clk/samsung/clk-exynos-audss.c +5 -0
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+     of_clk_del_provider(pdev->dev.of_node);
+
+     return 0;
 }

Don't we need to unregister all the registered clocks in remove? This also leads to another question: Do we even need removal support for this driver?

Agreed - I don't think we should support removal of this device, but it looks like __device_release_driver() just ignores the lack of a remove callback or the return value from remove. I suppose we could just yell that removal is not supported if it is ever attempted.

Quoted 9 lines

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 169

Quoted 5 lines
drivers/clk/samsung/clk-exynos-audss.c +5 -0
+static int __init exynos_audss_clk_init(void)
+{
+     return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);

Does it need to be core_initcall? Drivers depending on clocks provided by this driver should be able to defer probing if they are probed before this driver.

Unfortunately there are a couple of issues with making this a module_initcall:
1. On the Exynos5420, the AudioSS block provides the apb_pclk gate
for the ADMA bus, which is probed at postcore_initcall time and does not support deferred probing, and
2. the common clock framework doesn't differentiate between the
clock not being specified at all and the clock being specified, but the provider not being registered yet (i.e. the case where probe deferral would be appropriate) - it just returns ENOENT in both cases. Given that this driver has no dependencies other than the core clock controller, it should be fine to initialize at core_initcall time. Both of these issues could be fixed, though.

Thanks,
Andrew

#13 lore raw

PATCH 1/6 clk: exynos-audss: convert to platform device

Quoted 10 lines · On Monday 23 of September 2013 14:25:12 Andrew Bresticker wrote:
Quoted 9 lines

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 169

Quoted 5 lines
drivers/clk/samsung/clk-exynos-audss.c +5 -0
+static int __init exynos_audss_clk_init(void)
+{
+     return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);

Does it need to be core_initcall? Drivers depending on clocks provided by this driver should be able to defer probing if they are probed before this driver.

Quoted 4 lines

Unfortunately there are a couple of issues with making this a
module_initcall: 1. On the Exynos5420, the AudioSS block provides the apb_pclk gate for the ADMA bus, which is probed at postcore_initcall time and does not support deferred probing, and

Just out of curiosity, what is this ADMA bus?

Anyway, I'm fine with this, just wanted to make sure that there is a reason behind it.

Best regards,
Tomasz

#14 lore raw

PATCH 1/6 clk: exynos-audss: convert to platform device

Quoted 11 lines · Tomasz Figa
Quoted 10 lines · On Monday 23 of September 2013 14:25:12 Andrew Bresticker wrote:
Quoted 9 lines

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 169

Quoted 5 lines
drivers/clk/samsung/clk-exynos-audss.c +5 -0
+static int __init exynos_audss_clk_init(void)
+{
+     return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);

Does it need to be core_initcall? Drivers depending on clocks provided by this driver should be able to defer probing if they are probed before this driver.

Quoted 6 lines
Quoted 4 lines

Unfortunately there are a couple of issues with making this a
module_initcall: 1. On the Exynos5420, the AudioSS block provides the apb_pclk gate for the ADMA bus, which is probed at postcore_initcall time and does not support deferred probing, and

Just out of curiosity, what is this ADMA bus?

It's the DMA controller used for PCM and I2S. Looks like the DMA nodes aren't in the 5420 device-tree yet...

-Andrew

#15 lore raw
In reply to Andrew Bresticker

Hi,

Quoted 11 lines · Andrew Bresticker
Quoted 10 lines

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 147

Quoted 6 lines
drivers/clk/samsung/clk-exynos-audss.c +5 -0
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+     of_clk_del_provider(pdev->dev.of_node);
+
+     return 0;
  }

Don't we need to unregister all the registered clocks in remove? This also leads to another question: Do we even need removal support for this driver?

Quoted 4 lines

Agreed - I don't think we should support removal of this device, but it looks like __device_release_driver() just ignores the lack of a remove callback or the return value from remove. I suppose we could just yell that removal is not supported if it is ever attempted.

That might be a good idea, without proper remove() method deferred probing will also not work. I'd assume there should be only, e.g. WARN() in the remove() callback or it should be properly implemented, with clk_unregister() call for each currently registered clock.

Note that clk_unregister() is currently not implemented and removal of this driver cannot be properly supported at the moment anyway.

Not sure what's more appropriate, it's probably better to add clk_unregister() calls. This would be effectively a dead code though, as long as core_initcall is used.

Quoted 10 lines
Quoted 9 lines

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 169

Quoted 5 lines
drivers/clk/samsung/clk-exynos-audss.c +5 -0
+static int __init exynos_audss_clk_init(void)
+{
+     return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);

Does it need to be core_initcall? Drivers depending on clocks provided by this driver should be able to defer probing if they are probed before this driver.

Quoted 8 lines

Unfortunately there are a couple of issues with making this a module_initcall:
1. On the Exynos5420, the AudioSS block provides the apb_pclk gate
for the ADMA bus, which is probed at postcore_initcall time and does not support deferred probing, and
2. the common clock framework doesn't differentiate between the
clock not being specified at all and the clock being specified, but the provider not being registered yet (i.e. the case where probe deferral would be appropriate) - it just returns ENOENT in both cases.

AFAICS this shouldn't be difficult to improve. I guess it has not been properly addressed so far because there is currently no properly working modular clock provider drivers, using the common clock framework, yet. Unless someone bits me to it, I might have a look at that, as I also found it a bit it inconvenient.

--
Regards,
Sylwester

#16 lore raw

PATCH v2 1/6 clk: exynos-audss: convert to platform device

In reply to Andrew Bresticker

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.

---
Changes since v1:
- add clk_unregister() calls to remove callback
- fixed minor nits from Tomasz

---
 drivers/clk/samsung/clk-exynos-audss.c | 78 ++++++++++++++++++++++++++++------
 1 file changed, 65 insertions(+), 13 deletions(-)
drivers/clk/samsung/clk-exynos-audss.c +65 -13
index 39b40aa..c512efd 100644
@@ -14,6 +14,8 @@
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include <dt-bindings/clk/exynos-audss-clk.h>
 
@@ -62,24 +64,29 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 #endif /* CONFIG_PM_SLEEP */
 
 /* register exynos_audss clocks */
-static void __init exynos_audss_clk_init(struct device_node *np)
+static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
-	reg_base = of_iomap(np, 0);
-	if (!reg_base) {
-		pr_err("%s: failed to map audss registers\n", __func__);
-		return;
+	struct resource *res;
+
+	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 = kzalloc(sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
+	clk_table = devm_kzalloc(&pdev->dev,
+				sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
 				GFP_KERNEL);
 	if (!clk_table) {
-		pr_err("%s: could not allocate clk lookup table\n", __func__);
-		return;
+		dev_err(&pdev->dev, "could not allocate clk lookup table\n");
+		return -ENOMEM;
 	}
 
 	clk_data.clks = clk_table;
 	clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
-	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+	of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
+				&clk_data);
 
 	clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
 				mout_audss_p, ARRAY_SIZE(mout_audss_p),
@@ -128,8 +135,53 @@ static void __init exynos_audss_clk_init(struct device_node *np)
 #endif
 
 	pr_info("Exynos: Audss: clock setup completed\n");
+
+	return 0;
+}
+
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+	int i;
+
+	for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (clk_table[i])
+			clk_unregister(clk_table[i]);
+	}
+
+	of_clk_del_provider(pdev->dev.of_node);
+
+	return 0;
 }
-CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
-		exynos_audss_clk_init);
-CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
-		exynos_audss_clk_init);
+
+static const struct of_device_id exynos_audss_clk_of_match[] = {
+	{ .compatible = "samsung,exynos4210-audss-clock", },
+	{ .compatible = "samsung,exynos5250-audss-clock", },
+	{},
+};
+
+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)
+{
+	return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);
+
+static void __init exynos_audss_clk_exit(void)
+{
+	platform_driver_unregister(&exynos_audss_clk_driver);
+}
+module_exit(exynos_audss_clk_exit);
+
+MODULE_AUTHOR("Padmavathi Venna <[email protected]>");
+MODULE_DESCRIPTION("Exynos Audio Subsystem Clock Controller");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:exynos-audss-clk");
Signature · 1 line
1.8.4
#17 lore raw

PATCH v2 2/6 clk: exynos-audss: allow input clocks to be specified in device tree

This allows the input clocks to the Exynos AudioSS block to be specified via device-tree bindings. Default names will be used when an input clock is not given.

---
Changes since v1:
- listed input clocks as required properties

---
 .../devicetree/bindings/clock/clk-exynos-audss.txt | 32 ++++++++++++++++++++--
 drivers/clk/samsung/clk-exynos-audss.c             | 25 +++++++++++++----
 2 files changed, 50 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +30 -2
index 75e2e19..85b9e28 100644
@@ -14,6 +14,21 @@ Required Properties:
 
 - #clock-cells: should be 1.
 
+- clocks:
+  - pll_ref: Fixed rate PLL reference clock, parent of mout_audss. "fin_pll"
+    is used if not specified.
+  - pll_in: Input PLL to the AudioSS block, parent of mout_audss. "fout_epll"
+    is used if not specified.
+  - cdclk: External i2s clock, parent of mout_i2s. "cdclk0" is used if not
+    specified.
+  - sclk_audio: Audio bus clock, parent of mout_i2s. "sclk_audio0" is used if
+    not specified.
+  - sclk_pcm_in: PCM clock, parent of sclk_pcm.  "sclk_pcm0" is used if not
+    specified.
+
+- clock-names: Aliases for the above clocks. They should be "pll_ref",
+  "pll_in", "cdclk", "sclk_audio", and "sclk_pcm_in" respectively.
+
 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
@@ -35,15 +50,28 @@ sclk_i2s        7
 pcm_bus         8
 sclk_pcm        9
 
-Example 1: An example of a clock controller node is listed below.
+Example 1: An example of a clock controller node using the default input
+	   clock names is listed below.
+
+clock_audss: audss-clock-controller at 3810000 {
+	compatible = "samsung,exynos5250-audss-clock";
+	reg = <0x03810000 0x0C>;
+	#clock-cells = <1>;
+};
+
+Example 2: An example of a clock controller node with the input clocks
+           specified.
 
 clock_audss: audss-clock-controller at 3810000 {
 	compatible = "samsung,exynos5250-audss-clock";
 	reg = <0x03810000 0x0C>;
 	#clock-cells = <1>;
+	clocks = <&clock 1>, <&clock 7>, <&clock 138>, <&clock 160>,
+		<&ext_i2s_clk>;
+	clock-names = "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in", "cdclk";
 };
 
-Example 2: I2S controller node that consumes the clock generated by the clock
+Example 3: 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.
 
drivers/clk/samsung/clk-exynos-audss.c +20 -5
index c512efd..afb53de 100644
@@ -34,10 +34,6 @@ static unsigned long reg_save[][2] = {
 	{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 exynos_audss_clk_suspend(void)
 {
@@ -66,6 +62,10 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 /* register exynos_audss clocks */
 static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
+	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;
 	struct resource *res;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -88,11 +88,23 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
 				&clk_data);
 
+	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);
 	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,
@@ -126,8 +138,11 @@ 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",
-				"div_pcm0", CLK_SET_RATE_PARENT,
+				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
 #ifdef CONFIG_PM_SLEEP
Signature · 1 line
1.8.4
#18 lore raw

PATCH v2 3/6 clk: exynos5250: add clock ID for div_pcm0

In reply to Andrew Bresticker

There is no gate for the PCM clock input to the AudioSS block, so the parent of sclk_pcm is div_pcm0. Add a clock ID for it so that we can reference it in device trees.

---
 Documentation/devicetree/bindings/clock/exynos5250-clock.txt | 1 +
 drivers/clk/samsung/clk-exynos5250.c                         | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/clock/exynos5250-clock.txt +1 -0
index 24765c1..67e9a47 100644
@@ -62,6 +62,7 @@ clock which they consume.
   div_i2s1		157
   div_i2s2		158
   sclk_hdmiphy		159
+  div_pcm0		160
 
 
    [Peripheral Clock Gates]
drivers/clk/samsung/clk-exynos5250.c +2 -2
index adf3234..dec5376 100644
@@ -108,7 +108,7 @@ enum exynos5250_clks {
 	sclk_mmc0, sclk_mmc1, sclk_mmc2, sclk_mmc3, sclk_sata, sclk_usb3,
 	sclk_jpeg, sclk_uart0, sclk_uart1, sclk_uart2, sclk_uart3, sclk_pwm,
 	sclk_audio1, sclk_audio2, sclk_spdif, sclk_spi0, sclk_spi1, sclk_spi2,
-	div_i2s1, div_i2s2, sclk_hdmiphy,
+	div_i2s1, div_i2s2, sclk_hdmiphy, div_pcm0,
 
 	/* gate clocks */
 	gscl0 = 256, gscl1, gscl2, gscl3, gscl_wa, gscl_wb, smmu_gscl0,
@@ -301,7 +301,7 @@ static struct samsung_div_clock exynos5250_div_clks[] __initdata = {
 	DIV(none, "div_dp", "mout_dp", DIV_DISP1_0, 24, 4),
 	DIV(none, "div_jpeg", "mout_jpeg", DIV_GEN, 4, 4),
 	DIV(none, "div_audio0", "mout_audio0", DIV_MAU, 0, 4),
-	DIV(none, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
+	DIV(div_pcm0, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
 	DIV(none, "div_sata", "mout_sata", DIV_FSYS0, 20, 4),
 	DIV(none, "div_usb3", "mout_usb3", DIV_FSYS0, 24, 4),
 	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
Signature · 1 line
1.8.4
#19 lore raw

PATCH v2 4/6 ARM: dts: exynos5250: add input clocks to audss clock controller

In reply to Andrew Bresticker

Specify pll_ref, pll_in, sclk_audio, and sclk_pcm_in for the AudioSS clock controller.

---
Changes since v1:
- specified additional input clocks

---
 arch/arm/boot/dts/exynos5250.dtsi | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/boot/dts/exynos5250.dtsi +2 -0
index 7d7cc77..35e2838 100644
@@ -88,6 +88,8 @@
 		compatible = "samsung,exynos5250-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
+		clocks = <&clock 1>, <&clock 7>, <&clock 138>, <&clock 160>;
+		clock-names = "pll_ref, "pll_in", "sclk_audio", "sclk_pcm_in";
 	};
 
 	timer {
Signature · 1 line
1.8.4
#20 lore raw

PATCH v2 5/6 clk: exynos-audss: add support for Exynos 5420

In reply to Andrew Bresticker

The AudioSS block on Exynos 5420 has an additional clock gate for the ADMA bus clock.

---
Changes since v1:
- added type enum and made comparison against that instead of compatibility string

---
 .../devicetree/bindings/clock/clk-exynos-audss.txt |  7 +++--
 drivers/clk/samsung/clk-exynos-audss.c             | 35 ++++++++++++++++++----
 include/dt-bindings/clk/exynos-audss-clk.h         |  3 +-
 3 files changed, 36 insertions(+), 9 deletions(-)
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +5 -2
index 85b9e28..180e883 100644
@@ -8,8 +8,10 @@ Required Properties:
 
 - compatible: should be one of the following:
   - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.
-  - "samsung,exynos5250-audss-clock" - controller compatible with all Exynos5 SoCs.
-
+  - "samsung,exynos5250-audss-clock" - controller compatible with Exynos5250
+    SoCs.
+  - "samsung,exynos5420-audss-clock" - controller compatible with Exynos5420
+    SoCs.
 - reg: physical base address and length of the controller's register set.
 
 - #clock-cells: should be 1.
@@ -49,6 +51,7 @@ i2s_bus         6
 sclk_i2s        7
 pcm_bus         8
 sclk_pcm        9
+adma            10      Exynos5420
 
 Example 1: An example of a clock controller node using the default input
 	   clock names is listed below.
drivers/clk/samsung/clk-exynos-audss.c +29 -6
index afb53de..8ccf3788 100644
@@ -19,6 +19,12 @@
 
 #include <dt-bindings/clk/exynos-audss-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;
@@ -59,6 +65,16 @@ 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)
 {
@@ -67,6 +83,13 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	const char *sclk_pcm_p = "sclk_pcm0";
 	struct clk *pll_ref, *pll_in, *cdclk, *sclk_audio, *sclk_pcm_in;
 	struct resource *res;
+	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);
@@ -145,6 +168,12 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
+	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);
+	}
+
 #ifdef CONFIG_PM_SLEEP
 	register_syscore_ops(&exynos_audss_clk_syscore_ops);
 #endif
@@ -168,12 +197,6 @@ static int exynos_audss_clk_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id exynos_audss_clk_of_match[] = {
-	{ .compatible = "samsung,exynos4210-audss-clock", },
-	{ .compatible = "samsung,exynos5250-audss-clock", },
-	{},
-};
-
 static struct platform_driver exynos_audss_clk_driver = {
 	.driver	= {
 		.name = "exynos-audss-clk",
include/dt-bindings/clk/exynos-audss-clk.h +2 -1
index 8279f42..0ae6f5a 100644
@@ -19,7 +19,8 @@
 #define EXYNOS_SCLK_I2S	7
 #define EXYNOS_PCM_BUS		8
 #define EXYNOS_SCLK_PCM	9
+#define EXYNOS_ADMA		10
 
-#define EXYNOS_AUDSS_MAX_CLKS	10
+#define EXYNOS_AUDSS_MAX_CLKS	11
 
 #endif
Signature · 1 line
1.8.4
#21 lore raw

PATCH v2 6/6 ARM: dts: exynos5420: add input clocks to audss clock controller

In reply to Andrew Bresticker

Specify the remaining input clocks (pll_ref, pll_in, and sclk_pcm_in) for the AudioSS clock controller.

---
Changes since v1:
- specified additional input clocks

---
 arch/arm/boot/dts/exynos5420.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/exynos5420.dtsi +2 -2
index d537cd7..056b55e 100644
@@ -72,8 +72,8 @@
 		compatible = "samsung,exynos5420-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
-		clocks = <&clock 148>;
-		clock-names = "sclk_audio";
+		clocks = <&clock 1>, <&clock 5>, <&clock 148>, <&clock 149>;
+		clock-names = "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in";
 	};
 
 	codec at 11000000 {
Signature · 1 line
1.8.4
#22 lore raw
In reply to Andrew Bresticker

Hi Andrew,

I'd like to ack this series, but there is one more thing that I think should be fixed. Please see my comment inline.

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 137

Quoted 4 lines · On Monday 23 of September 2013 17:21:13 Andrew Bresticker wrote:
drivers/clk/samsung/clk-exynos-audss.c +0 -0
@@ -128,8 +135,53 @@ static void __init exynos_audss_clk_init(struct device_node *np)
 #endif
 
 	pr_info("Exynos: Audss: clock setup completed\n");

nit (not the thing I mentioned above): This (and possibly other uses of pr_*() could be replaced with dev_*().

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 147

Quoted 10 lines
drivers/clk/samsung/clk-exynos-audss.c +10 -0
+
+	return 0;
+}
+
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+	int i;
+
+	for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (clk_table[i])

I believe clk_register_* functions return ERR_PTR() in case of failure, not NULL, so this should be accounted for either here or at probe time. Possibly checking for registration error in probe() would be the best solution, although bloating the code a bit (but what error path isn't?).

Best regards,
Tomasz

#23 lore raw

PATCH v2 1/6 clk: exynos-audss: convert to platform device

Quoted 11 lines · Tomasz Figa

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 147

Quoted 6 lines
drivers/clk/samsung/clk-exynos-audss.c +6 -0
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+	int i;
+
+	for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (clk_table[i])

I believe clk_register_* functions return ERR_PTR() in case of failure, not NULL, so this should be accounted for either here or at probe time. Possibly checking for registration error in probe() would be the best solution, although bloating the code a bit (but what error path isn't?).

After registering all clocks a loop iterating over all entries of the clk_table[] array could be added and if any of the entries is ERR_PTR() either an error could be just logged or the successfully registered clocks could be freed and probe() could fail. It doesn't really seems much additional code.

--
Thanks,
Sylwester

#24 lore raw
In reply to Andrew Bresticker

Hi,

Quoted 18 lines · Andrew Bresticker
drivers/clk/samsung/clk-exynos-audss.c +8 -5
@@ -62,24 +64,29 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 #endif /* CONFIG_PM_SLEEP */
 
 /* register exynos_audss clocks */
-static void __init exynos_audss_clk_init(struct device_node *np)
+static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
-	reg_base = of_iomap(np, 0);
-	if (!reg_base) {
-		pr_err("%s: failed to map audss registers\n", __func__);
-		return;
+	struct resource *res;
+
+	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);

drivers/clk/samsung/clk-exynos-audss.c static struct syscore_ops exynos_audss_clk_syscore_ops = { · line 82

Quoted 10 lines
drivers/clk/samsung/clk-exynos-audss.c +3 -3
 	}
 
-	clk_table = kzalloc(sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
+	clk_table = devm_kzalloc(&pdev->dev,
+				sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
 				GFP_KERNEL);
 	if (!clk_table) {
-		pr_err("%s: could not allocate clk lookup table\n", __func__);
-		return;
+		dev_err(&pdev->dev, "could not allocate clk lookup table\n");

You could drop this error log, k*alloc() functions already log any errors.

Quoted 15 lines
drivers/clk/samsung/clk-exynos-audss.c +3 -1
+		return -ENOMEM;
 	}
 
 	clk_data.clks = clk_table;
 	clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
-	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+	of_clk_add_provider(pdev->dev.of_node, of_clk_src_onecell_get,
+				&clk_data);
 
 	clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
 				mout_audss_p, ARRAY_SIZE(mout_audss_p),
@@ -128,8 +135,53 @@ static void __init exynos_audss_clk_init(struct device_node *np)
 #endif
 
 	pr_info("Exynos: Audss: clock setup completed\n");

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 147

Quoted 10 lines
drivers/clk/samsung/clk-exynos-audss.c +10 -0
+
+	return 0;
+}
+
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+	int i;
+
+	for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (clk_table[i])

This would need to be:

		if (!IS_ERR(clk_table[i]))

Note the clock provider should be unregistered first, to avoid potential race condition, where the clock provider returns an invalid pointer to a clock, which has already been unregistered and freed.

I just noticed the sequence in probe needs to be fixed as well. i.e. clocks should be created with clk_register() before the clock provider is actually registered. It might warrant a separate patch but it's probably also fine to make such change part of this patch.

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 154

Quoted 7 lines
drivers/clk/samsung/clk-exynos-audss.c +6 -0
+			clk_unregister(clk_table[i]);
+	}
+
+	of_clk_del_provider(pdev->dev.of_node);
+
+	return 0;
 }

--
Thanks,
Sylwester

#25 lore raw

PATCH v3 1/6 clk: exynos-audss: convert to platform device

In reply to Andrew Bresticker

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.

---
Changes since v2:
- add error handling to probe callback
- fixed ordering of of_clk_{add,del}_provider
- fixed nits from Tomasz and Sylwester
Changes since v1:
- add clk_unregister() calls to remove callback
- fixed minor nits from Tomasz

---
 drivers/clk/samsung/clk-exynos-audss.c | 109 +++++++++++++++++++++++++++------
 1 file changed, 90 insertions(+), 19 deletions(-)
drivers/clk/samsung/clk-exynos-audss.c +90 -19
index 39b40aa..319c6e4 100644
@@ -14,6 +14,8 @@
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include <dt-bindings/clk/exynos-audss-clk.h>
 
@@ -62,24 +64,23 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 #endif /* CONFIG_PM_SLEEP */
 
 /* register exynos_audss clocks */
-static void __init exynos_audss_clk_init(struct device_node *np)
+static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
-	reg_base = of_iomap(np, 0);
-	if (!reg_base) {
-		pr_err("%s: failed to map audss registers\n", __func__);
-		return;
+	int i, ret = 0;
+	struct resource *res;
+
+	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 = kzalloc(sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
+	clk_table = devm_kzalloc(&pdev->dev,
+				sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
 				GFP_KERNEL);
-	if (!clk_table) {
-		pr_err("%s: could not allocate clk lookup table\n", __func__);
-		return;
-	}
-
-	clk_data.clks = clk_table;
-	clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
-	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+	if (!clk_table)
+		return -ENOMEM;
 
 	clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
 				mout_audss_p, ARRAY_SIZE(mout_audss_p),
@@ -123,13 +124,83 @@ static void __init exynos_audss_clk_init(struct device_node *np)
 				"div_pcm0", CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
+	for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (IS_ERR(clk_table[i])) {
+			dev_err(&pdev->dev, "failed to regsiter clock %d\n", i);
+			ret = PTR_ERR(clk_table[i]);
+			goto unregister;
+		}
+	}
+
+	clk_data.clks = clk_table;
+	clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
+	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;
+	}
+
 #ifdef CONFIG_PM_SLEEP
 	register_syscore_ops(&exynos_audss_clk_syscore_ops);
 #endif
 
-	pr_info("Exynos: Audss: clock setup completed\n");
+	dev_info(&pdev->dev, "setup completed\n");
+
+	return 0;
+
+unregister:
+	for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (!IS_ERR_OR_NULL(clk_table[i]))
+			clk_unregister(clk_table[i]);
+	}
+
+	return ret;
 }
-CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
-		exynos_audss_clk_init);
-CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
-		exynos_audss_clk_init);
+
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+	int i;
+
+	of_clk_del_provider(pdev->dev.of_node);
+
+	for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (!IS_ERR_OR_NULL(clk_table[i]))
+			clk_unregister(clk_table[i]);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id exynos_audss_clk_of_match[] = {
+	{ .compatible = "samsung,exynos4210-audss-clock", },
+	{ .compatible = "samsung,exynos5250-audss-clock", },
+	{},
+};
+
+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)
+{
+	return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);
+
+static void __init exynos_audss_clk_exit(void)
+{
+	platform_driver_unregister(&exynos_audss_clk_driver);
+}
+module_exit(exynos_audss_clk_exit);
+
+MODULE_AUTHOR("Padmavathi Venna <[email protected]>");
+MODULE_DESCRIPTION("Exynos Audio Subsystem Clock Controller");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:exynos-audss-clk");
Signature · 1 line
1.8.4
#26 lore raw

PATCH v3 2/6 clk: exynos-audss: allow input clocks to be specified in device tree

This allows the input clocks to the Exynos AudioSS block to be specified via device-tree bindings. Default names will be used when an input clock is not given.

---
Changes since v1:
- listed input clocks as required properties

---
 .../devicetree/bindings/clock/clk-exynos-audss.txt | 32 ++++++++++++++++++++--
 drivers/clk/samsung/clk-exynos-audss.c             | 25 +++++++++++++----
 2 files changed, 50 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +30 -2
index 75e2e19..85b9e28 100644
@@ -14,6 +14,21 @@ Required Properties:
 
 - #clock-cells: should be 1.
 
+- clocks:
+  - pll_ref: Fixed rate PLL reference clock, parent of mout_audss. "fin_pll"
+    is used if not specified.
+  - pll_in: Input PLL to the AudioSS block, parent of mout_audss. "fout_epll"
+    is used if not specified.
+  - cdclk: External i2s clock, parent of mout_i2s. "cdclk0" is used if not
+    specified.
+  - sclk_audio: Audio bus clock, parent of mout_i2s. "sclk_audio0" is used if
+    not specified.
+  - sclk_pcm_in: PCM clock, parent of sclk_pcm.  "sclk_pcm0" is used if not
+    specified.
+
+- clock-names: Aliases for the above clocks. They should be "pll_ref",
+  "pll_in", "cdclk", "sclk_audio", and "sclk_pcm_in" respectively.
+
 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
@@ -35,15 +50,28 @@ sclk_i2s        7
 pcm_bus         8
 sclk_pcm        9
 
-Example 1: An example of a clock controller node is listed below.
+Example 1: An example of a clock controller node using the default input
+	   clock names is listed below.
+
+clock_audss: audss-clock-controller at 3810000 {
+	compatible = "samsung,exynos5250-audss-clock";
+	reg = <0x03810000 0x0C>;
+	#clock-cells = <1>;
+};
+
+Example 2: An example of a clock controller node with the input clocks
+           specified.
 
 clock_audss: audss-clock-controller at 3810000 {
 	compatible = "samsung,exynos5250-audss-clock";
 	reg = <0x03810000 0x0C>;
 	#clock-cells = <1>;
+	clocks = <&clock 1>, <&clock 7>, <&clock 138>, <&clock 160>,
+		<&ext_i2s_clk>;
+	clock-names = "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in", "cdclk";
 };
 
-Example 2: I2S controller node that consumes the clock generated by the clock
+Example 3: 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.
 
drivers/clk/samsung/clk-exynos-audss.c +20 -5
index 319c6e4..e7e800a 100644
@@ -34,10 +34,6 @@ static unsigned long reg_save[][2] = {
 	{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 exynos_audss_clk_suspend(void)
 {
@@ -68,6 +64,10 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
 	int i, ret = 0;
 	struct resource *res;
+	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;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	reg_base = devm_ioremap_resource(&pdev->dev, res);
@@ -82,11 +82,23 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	if (!clk_table)
 		return -ENOMEM;
 
+	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);
 	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,
@@ -120,8 +132,11 @@ 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",
-				"div_pcm0", CLK_SET_RATE_PARENT,
+				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
 	for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++) {
Signature · 1 line
1.8.4
#27 lore raw

PATCH v3 3/6 clk: exynos5250: add clock ID for div_pcm0

In reply to Andrew Bresticker

There is no gate for the PCM clock input to the AudioSS block, so the parent of sclk_pcm is div_pcm0. Add a clock ID for it so that we can reference it in device trees.

---
 Documentation/devicetree/bindings/clock/exynos5250-clock.txt | 1 +
 drivers/clk/samsung/clk-exynos5250.c                         | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/clock/exynos5250-clock.txt +1 -0
index 24765c1..67e9a47 100644
@@ -62,6 +62,7 @@ clock which they consume.
   div_i2s1		157
   div_i2s2		158
   sclk_hdmiphy		159
+  div_pcm0		160
 
 
    [Peripheral Clock Gates]
drivers/clk/samsung/clk-exynos5250.c +2 -2
index adf3234..dec5376 100644
@@ -108,7 +108,7 @@ enum exynos5250_clks {
 	sclk_mmc0, sclk_mmc1, sclk_mmc2, sclk_mmc3, sclk_sata, sclk_usb3,
 	sclk_jpeg, sclk_uart0, sclk_uart1, sclk_uart2, sclk_uart3, sclk_pwm,
 	sclk_audio1, sclk_audio2, sclk_spdif, sclk_spi0, sclk_spi1, sclk_spi2,
-	div_i2s1, div_i2s2, sclk_hdmiphy,
+	div_i2s1, div_i2s2, sclk_hdmiphy, div_pcm0,
 
 	/* gate clocks */
 	gscl0 = 256, gscl1, gscl2, gscl3, gscl_wa, gscl_wb, smmu_gscl0,
@@ -301,7 +301,7 @@ static struct samsung_div_clock exynos5250_div_clks[] __initdata = {
 	DIV(none, "div_dp", "mout_dp", DIV_DISP1_0, 24, 4),
 	DIV(none, "div_jpeg", "mout_jpeg", DIV_GEN, 4, 4),
 	DIV(none, "div_audio0", "mout_audio0", DIV_MAU, 0, 4),
-	DIV(none, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
+	DIV(div_pcm0, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
 	DIV(none, "div_sata", "mout_sata", DIV_FSYS0, 20, 4),
 	DIV(none, "div_usb3", "mout_usb3", DIV_FSYS0, 24, 4),
 	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
Signature · 1 line
1.8.4
#28 lore raw

PATCH v3 4/6 ARM: dts: exynos5250: add input clocks to audss clock controller

In reply to Andrew Bresticker

Specify pll_ref, pll_in, sclk_audio, and sclk_pcm_in for the AudioSS clock controller.

---
Changes since v1:
- specified additional input clocks

---
 arch/arm/boot/dts/exynos5250.dtsi | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/boot/dts/exynos5250.dtsi +2 -0
index 7d7cc77..2d6a93d 100644
@@ -88,6 +88,8 @@
 		compatible = "samsung,exynos5250-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
+		clocks = <&clock 1>, <&clock 7>, <&clock 138>, <&clock 160>;
+		clock-names = "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in";
 	};
 
 	timer {
Signature · 1 line
1.8.4
#29 lore raw

PATCH v3 5/6 clk: exynos-audss: add support for Exynos 5420

In reply to Andrew Bresticker

The AudioSS block on Exynos 5420 has an additional clock gate for the ADMA bus clock.

---
Changes since v1:
- added type enum and made comparison against that instead of compatibility string

---
 .../devicetree/bindings/clock/clk-exynos-audss.txt |  7 +++--
 drivers/clk/samsung/clk-exynos-audss.c             | 35 ++++++++++++++++++----
 include/dt-bindings/clk/exynos-audss-clk.h         |  3 +-
 3 files changed, 36 insertions(+), 9 deletions(-)
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +5 -2
index 85b9e28..180e883 100644
@@ -8,8 +8,10 @@ Required Properties:
 
 - compatible: should be one of the following:
   - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.
-  - "samsung,exynos5250-audss-clock" - controller compatible with all Exynos5 SoCs.
-
+  - "samsung,exynos5250-audss-clock" - controller compatible with Exynos5250
+    SoCs.
+  - "samsung,exynos5420-audss-clock" - controller compatible with Exynos5420
+    SoCs.
 - reg: physical base address and length of the controller's register set.
 
 - #clock-cells: should be 1.
@@ -49,6 +51,7 @@ i2s_bus         6
 sclk_i2s        7
 pcm_bus         8
 sclk_pcm        9
+adma            10      Exynos5420
 
 Example 1: An example of a clock controller node using the default input
 	   clock names is listed below.
drivers/clk/samsung/clk-exynos-audss.c +29 -6
index e7e800a..35cae41 100644
@@ -19,6 +19,12 @@
 
 #include <dt-bindings/clk/exynos-audss-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;
@@ -59,6 +65,16 @@ 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)
 {
@@ -68,6 +84,13 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	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;
+
+	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);
@@ -139,6 +162,12 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
+	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 < EXYNOS_AUDSS_MAX_CLKS; i++) {
 		if (IS_ERR(clk_table[i])) {
 			dev_err(&pdev->dev, "failed to regsiter clock %d\n", i);
@@ -187,12 +216,6 @@ static int exynos_audss_clk_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id exynos_audss_clk_of_match[] = {
-	{ .compatible = "samsung,exynos4210-audss-clock", },
-	{ .compatible = "samsung,exynos5250-audss-clock", },
-	{},
-};
-
 static struct platform_driver exynos_audss_clk_driver = {
 	.driver	= {
 		.name = "exynos-audss-clk",
include/dt-bindings/clk/exynos-audss-clk.h +2 -1
index 8279f42..0ae6f5a 100644
@@ -19,7 +19,8 @@
 #define EXYNOS_SCLK_I2S	7
 #define EXYNOS_PCM_BUS		8
 #define EXYNOS_SCLK_PCM	9
+#define EXYNOS_ADMA		10
 
-#define EXYNOS_AUDSS_MAX_CLKS	10
+#define EXYNOS_AUDSS_MAX_CLKS	11
 
 #endif
Signature · 1 line
1.8.4
#30 lore raw

PATCH v3 6/6 ARM: dts: exynos5420: add input clocks to audss clock controller

In reply to Andrew Bresticker

Specify the remaining input clocks (pll_ref, pll_in, and sclk_pcm_in) for the AudioSS clock controller.

---
Changes since v1:
- specified additional input clocks

---
 arch/arm/boot/dts/exynos5420.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/exynos5420.dtsi +2 -2
index d537cd7..056b55e 100644
@@ -72,8 +72,8 @@
 		compatible = "samsung,exynos5420-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
-		clocks = <&clock 148>;
-		clock-names = "sclk_audio";
+		clocks = <&clock 1>, <&clock 5>, <&clock 148>, <&clock 149>;
+		clock-names = "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in";
 	};
 
 	codec at 11000000 {
Signature · 1 line
1.8.4
Acked-by on v3 1/6
#31 lore raw
In reply to Andrew Bresticker

Hi Andrew,

Quoted 18 lines · On Tuesday 24 of September 2013 11:06:51 Andrew Bresticker wrote:

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.

---
Changes since v2:
- add error handling to probe callback
- fixed ordering of of_clk_{add,del}_provider
- fixed nits from Tomasz and Sylwester
Changes since v1:
- add clk_unregister() calls to remove callback
- fixed minor nits from Tomasz

Quoted 4 lines
---
 drivers/clk/samsung/clk-exynos-audss.c | 109

+++++++++++++++++++++++++++------ 1 file changed, 90 insertions(+), 19 deletions(-)

For the whole series:

Thanks for addressing all the comments.

Best regards,
Tomasz

Reviewed-by on v3 1/6
#32 lore raw
In reply to Andrew Bresticker
Quoted 2 lines · Andrew Bresticker

+static int exynos_audss_clk_probe(struct platform_device *pdev)
{

[...]

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 129

Quoted 9 lines

clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
mout_audss_p, ARRAY_SIZE(mout_audss_p),

drivers/clk/samsung/clk-exynos-audss.c +3 -0
@@ -123,13 +124,83 @@ static void __init exynos_audss_clk_init(struct device_node *np)
  				"div_pcm0", CLK_SET_RATE_PARENT,
  				reg_base + ASS_CLK_GATE, 5, 0,&lock);
 
+	for (i = 0; i<  EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (IS_ERR(clk_table[i])) {
+			dev_err(&pdev->dev, "failed to regsiter clock %d\n", i);

typo: regsiter -> register

Quoted 14 lines
drivers/clk/samsung/clk-exynos-audss.c +14 -0
+			ret = PTR_ERR(clk_table[i]);
+			goto unregister;
+		}
+	}
+
+	clk_data.clks = clk_table;
+	clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
+	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;
+	}
+

[...]

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 156

Quoted 7 lines
drivers/clk/samsung/clk-exynos-audss.c +7 -0
+	return 0;
+
+unregister:
+	for (i = 0; i<  EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (!IS_ERR_OR_NULL(clk_table[i]))
+			clk_unregister(clk_table[i]);
+	}

Couldn't this instead be:

	while (--i >= 0)
		clk_unregister(clk_table[i]);

?

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 170

Quoted 10 lines
drivers/clk/samsung/clk-exynos-audss.c +10 -0
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+	int i;
+
+	of_clk_del_provider(pdev->dev.of_node);
+
+	for (i = 0; i<  EXYNOS_AUDSS_MAX_CLKS; i++) {
+		if (!IS_ERR_OR_NULL(clk_table[i]))
+			clk_unregister(clk_table[i]);
+	}

Since we only get here if all the clocks are registered properly and we always register EXYNOS_AUDSS_MAX_CLKS clocks, couldn't this simply be:

	for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++)
		clk_unregister(clk_table[i]);

?

Review

Quoted 2 lines
(quoted diff) +2 -0
+	return 0;
+}

Otherwise the whole series looks good to me. Feel free to add:

--
Thanks,
Sylwester

#33 lore raw
Quoted 16 lines

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 170

Quoted 10 lines
drivers/clk/samsung/clk-exynos-audss.c +10 -0
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+       int i;
+
+       of_clk_del_provider(pdev->dev.of_node);
+
+       for (i = 0; i<  EXYNOS_AUDSS_MAX_CLKS; i++) {
+               if (!IS_ERR_OR_NULL(clk_table[i]))
+                       clk_unregister(clk_table[i]);
+       }

Since we only get here if all the clocks are registered properly and we always register EXYNOS_AUDSS_MAX_CLKS clocks, couldn't this simply be:

Quoted 4 lines
        for (i = 0; i < EXYNOS_AUDSS_MAX_CLKS; i++)
                clk_unregister(clk_table[i]);

?

Once support is added for Exynos5420, we won't always register EXYNOS_AUDSS_MAX_CLKS clocks, so we'd still need the NULL check.

Thanks,
Andrew

A message that is not in the archive (sent off-list or not yet received): CAL1qeaEpu=YRasZpSvrCTNwC6OGWZgECjwDSFgkAN07eWObmrg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#35 lore raw
Quoted 16 lines · Andrew Bresticker
Quoted 16 lines

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 170

Quoted 10 lines
drivers/clk/samsung/clk-exynos-audss.c +10 -0
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+       int i;
+
+       of_clk_del_provider(pdev->dev.of_node);
+
+       for (i = 0; i<   EXYNOS_AUDSS_MAX_CLKS; i++) {
+               if (!IS_ERR_OR_NULL(clk_table[i]))
+                       clk_unregister(clk_table[i]);
+       }

Since we only get here if all the clocks are registered properly and we always register EXYNOS_AUDSS_MAX_CLKS clocks, couldn't this simply be:

Quoted 7 lines
Quoted 4 lines
         for (i = 0; i<  EXYNOS_AUDSS_MAX_CLKS; i++)
                 clk_unregister(clk_table[i]);

?

Once support is added for Exynos5420, we won't always register EXYNOS_AUDSS_MAX_CLKS clocks, so we'd still need the NULL check.

Then couldn't you use clk_data.clk_num in this loop instead of EXYNOS_AUDSS_MAX_CLKS ? What I'm trying to avoid is assuming NULL as an invalid clock. It's not correct, clock pointers should be only tested with ERR_PTR(). Alternatively you could set the whole clk_table array to, e.g. ERR_PTR(-EINVAL) beforehand, but that shouldn't be necessary when clk_data.clk_num is used, unless there are holes in the array.

--
Regards,
Sylwester

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#36 lore raw
In reply to Andrew Bresticker

drivers/clk/samsung/clk-exynos-audss.c static void __init exynos_audss_clk_init(struct device_node *np) · line 197

Quoted 7 lines · Andrew Bresticker
drivers/clk/samsung/clk-exynos-audss.c +7 -0
+static int __init exynos_audss_clk_init(void)
+{
+	return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);
+
+static void __init exynos_audss_clk_exit(void)

__exit?

Quoted 5 lines
drivers/clk/samsung/clk-exynos-audss.c +4 -0
+{
+	platform_driver_unregister(&exynos_audss_clk_driver);
+}
+module_exit(exynos_audss_clk_exit);
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
#37 lore raw

PATCH v4 1/6 clk: exynos-audss: convert to platform device

In reply to Andrew Bresticker

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.

---
Changes since v3:
- __init -> __exit for module exit function
- fixed nits from Sylwester
Changes since v2:
- add error handling to probe callback
- fixed ordering of of_clk_{add,del}_provider
- fixed nits from Tomasz and Sylwester
Changes since v1:
- add clk_unregister() calls to remove callback
- fixed minor nits from Tomasz

---
 drivers/clk/samsung/clk-exynos-audss.c | 104 ++++++++++++++++++++++++++++-----
 1 file changed, 88 insertions(+), 16 deletions(-)
drivers/clk/samsung/clk-exynos-audss.c +88 -16
index 39b40aa..742dabc 100644
@@ -14,6 +14,8 @@
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include <dt-bindings/clk/exynos-audss-clk.h>
 
@@ -62,24 +64,26 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 #endif /* CONFIG_PM_SLEEP */
 
 /* register exynos_audss clocks */
-static void __init exynos_audss_clk_init(struct device_node *np)
+static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
-	reg_base = of_iomap(np, 0);
-	if (!reg_base) {
-		pr_err("%s: failed to map audss registers\n", __func__);
-		return;
+	int i, ret = 0;
+	struct resource *res;
+
+	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 = kzalloc(sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
+	clk_table = devm_kzalloc(&pdev->dev,
+				sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
 				GFP_KERNEL);
-	if (!clk_table) {
-		pr_err("%s: could not allocate clk lookup table\n", __func__);
-		return;
-	}
+	if (!clk_table)
+		return -ENOMEM;
 
 	clk_data.clks = clk_table;
 	clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
-	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 
 	clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
 				mout_audss_p, ARRAY_SIZE(mout_audss_p),
@@ -123,13 +127,81 @@ static void __init exynos_audss_clk_init(struct device_node *np)
 				"div_pcm0", CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 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;
+	}
+
 #ifdef CONFIG_PM_SLEEP
 	register_syscore_ops(&exynos_audss_clk_syscore_ops);
 #endif
 
-	pr_info("Exynos: Audss: clock setup completed\n");
+	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;
+}
+
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+	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;
 }
-CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
-		exynos_audss_clk_init);
-CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
-		exynos_audss_clk_init);
+
+static const struct of_device_id exynos_audss_clk_of_match[] = {
+	{ .compatible = "samsung,exynos4210-audss-clock", },
+	{ .compatible = "samsung,exynos5250-audss-clock", },
+	{},
+};
+
+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)
+{
+	return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);
+
+static void __exit exynos_audss_clk_exit(void)
+{
+	platform_driver_unregister(&exynos_audss_clk_driver);
+}
+module_exit(exynos_audss_clk_exit);
+
+MODULE_AUTHOR("Padmavathi Venna <[email protected]>");
+MODULE_DESCRIPTION("Exynos Audio Subsystem Clock Controller");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:exynos-audss-clk");
Signature · 1 line
1.8.4
#38 lore raw

PATCH v4 2/6 clk: exynos-audss: allow input clocks to be specified in device tree

This allows the input clocks to the Exynos AudioSS block to be specified via device-tree bindings. Default names will be used when an input clock is not given.

---
Changes since v1:
- listed input clocks as required properties

---
 .../devicetree/bindings/clock/clk-exynos-audss.txt | 32 ++++++++++++++++++++--
 drivers/clk/samsung/clk-exynos-audss.c             | 25 +++++++++++++----
 2 files changed, 50 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +30 -2
index 75e2e19..85b9e28 100644
@@ -14,6 +14,21 @@ Required Properties:
 
 - #clock-cells: should be 1.
 
+- clocks:
+  - pll_ref: Fixed rate PLL reference clock, parent of mout_audss. "fin_pll"
+    is used if not specified.
+  - pll_in: Input PLL to the AudioSS block, parent of mout_audss. "fout_epll"
+    is used if not specified.
+  - cdclk: External i2s clock, parent of mout_i2s. "cdclk0" is used if not
+    specified.
+  - sclk_audio: Audio bus clock, parent of mout_i2s. "sclk_audio0" is used if
+    not specified.
+  - sclk_pcm_in: PCM clock, parent of sclk_pcm.  "sclk_pcm0" is used if not
+    specified.
+
+- clock-names: Aliases for the above clocks. They should be "pll_ref",
+  "pll_in", "cdclk", "sclk_audio", and "sclk_pcm_in" respectively.
+
 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
@@ -35,15 +50,28 @@ sclk_i2s        7
 pcm_bus         8
 sclk_pcm        9
 
-Example 1: An example of a clock controller node is listed below.
+Example 1: An example of a clock controller node using the default input
+	   clock names is listed below.
+
+clock_audss: audss-clock-controller at 3810000 {
+	compatible = "samsung,exynos5250-audss-clock";
+	reg = <0x03810000 0x0C>;
+	#clock-cells = <1>;
+};
+
+Example 2: An example of a clock controller node with the input clocks
+           specified.
 
 clock_audss: audss-clock-controller at 3810000 {
 	compatible = "samsung,exynos5250-audss-clock";
 	reg = <0x03810000 0x0C>;
 	#clock-cells = <1>;
+	clocks = <&clock 1>, <&clock 7>, <&clock 138>, <&clock 160>,
+		<&ext_i2s_clk>;
+	clock-names = "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in", "cdclk";
 };
 
-Example 2: I2S controller node that consumes the clock generated by the clock
+Example 3: 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.
 
drivers/clk/samsung/clk-exynos-audss.c +20 -5
index 742dabc..7cb10f2 100644
@@ -34,10 +34,6 @@ static unsigned long reg_save[][2] = {
 	{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 exynos_audss_clk_suspend(void)
 {
@@ -68,6 +64,10 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
 	int i, ret = 0;
 	struct resource *res;
+	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;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	reg_base = devm_ioremap_resource(&pdev->dev, res);
@@ -85,11 +85,23 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	clk_data.clks = clk_table;
 	clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
 
+	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);
 	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,
@@ -123,8 +135,11 @@ 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",
-				"div_pcm0", CLK_SET_RATE_PARENT,
+				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
 	for (i = 0; i < clk_data.clk_num; i++) {
Signature · 1 line
1.8.4
#39 lore raw

PATCH v4 3/6 clk: exynos5250: add clock ID for div_pcm0

In reply to Andrew Bresticker

There is no gate for the PCM clock input to the AudioSS block, so the parent of sclk_pcm is div_pcm0. Add a clock ID for it so that we can reference it in device trees.

---
 Documentation/devicetree/bindings/clock/exynos5250-clock.txt | 1 +
 drivers/clk/samsung/clk-exynos5250.c                         | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/clock/exynos5250-clock.txt +1 -0
index 24765c1..67e9a47 100644
@@ -62,6 +62,7 @@ clock which they consume.
   div_i2s1		157
   div_i2s2		158
   sclk_hdmiphy		159
+  div_pcm0		160
 
 
    [Peripheral Clock Gates]
drivers/clk/samsung/clk-exynos5250.c +2 -2
index adf3234..dec5376 100644
@@ -108,7 +108,7 @@ enum exynos5250_clks {
 	sclk_mmc0, sclk_mmc1, sclk_mmc2, sclk_mmc3, sclk_sata, sclk_usb3,
 	sclk_jpeg, sclk_uart0, sclk_uart1, sclk_uart2, sclk_uart3, sclk_pwm,
 	sclk_audio1, sclk_audio2, sclk_spdif, sclk_spi0, sclk_spi1, sclk_spi2,
-	div_i2s1, div_i2s2, sclk_hdmiphy,
+	div_i2s1, div_i2s2, sclk_hdmiphy, div_pcm0,
 
 	/* gate clocks */
 	gscl0 = 256, gscl1, gscl2, gscl3, gscl_wa, gscl_wb, smmu_gscl0,
@@ -301,7 +301,7 @@ static struct samsung_div_clock exynos5250_div_clks[] __initdata = {
 	DIV(none, "div_dp", "mout_dp", DIV_DISP1_0, 24, 4),
 	DIV(none, "div_jpeg", "mout_jpeg", DIV_GEN, 4, 4),
 	DIV(none, "div_audio0", "mout_audio0", DIV_MAU, 0, 4),
-	DIV(none, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
+	DIV(div_pcm0, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
 	DIV(none, "div_sata", "mout_sata", DIV_FSYS0, 20, 4),
 	DIV(none, "div_usb3", "mout_usb3", DIV_FSYS0, 24, 4),
 	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
Signature · 1 line
1.8.4
#40 lore raw

PATCH v4 4/6 ARM: dts: exynos5250: add input clocks to audss clock controller

In reply to Andrew Bresticker

Specify pll_ref, pll_in, sclk_audio, and sclk_pcm_in for the AudioSS clock controller.

---
Changes since v1:
- specified additional input clocks

---
 arch/arm/boot/dts/exynos5250.dtsi | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/boot/dts/exynos5250.dtsi +2 -0
index 7d7cc77..2d6a93d 100644
@@ -88,6 +88,8 @@
 		compatible = "samsung,exynos5250-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
+		clocks = <&clock 1>, <&clock 7>, <&clock 138>, <&clock 160>;
+		clock-names = "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in";
 	};
 
 	timer {
Signature · 1 line
1.8.4
#41 lore raw

PATCH v4 5/6 clk: exynos-audss: add support for Exynos 5420

In reply to Andrew Bresticker

The AudioSS block on Exynos 5420 has an additional clock gate for the ADMA bus clock.

---
Changes since v3:
- set clk_data.clk_num to correct value in non-5420 case
Changes since v1:
- added type enum and made comparison against that instead of compatibility string

---
 .../devicetree/bindings/clock/clk-exynos-audss.txt |  7 ++--
 drivers/clk/samsung/clk-exynos-audss.c             | 40 ++++++++++++++++++----
 include/dt-bindings/clk/exynos-audss-clk.h         |  3 +-
 3 files changed, 40 insertions(+), 10 deletions(-)
Documentation/devicetree/bindings/clock/clk-exynos-audss.txt +5 -2
index 85b9e28..180e883 100644
@@ -8,8 +8,10 @@ Required Properties:
 
 - compatible: should be one of the following:
   - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.
-  - "samsung,exynos5250-audss-clock" - controller compatible with all Exynos5 SoCs.
-
+  - "samsung,exynos5250-audss-clock" - controller compatible with Exynos5250
+    SoCs.
+  - "samsung,exynos5420-audss-clock" - controller compatible with Exynos5420
+    SoCs.
 - reg: physical base address and length of the controller's register set.
 
 - #clock-cells: should be 1.
@@ -49,6 +51,7 @@ i2s_bus         6
 sclk_i2s        7
 pcm_bus         8
 sclk_pcm        9
+adma            10      Exynos5420
 
 Example 1: An example of a clock controller node using the default input
 	   clock names is listed below.
drivers/clk/samsung/clk-exynos-audss.c +33 -7
index 7cb10f2..e607176 100644
@@ -19,6 +19,12 @@
 
 #include <dt-bindings/clk/exynos-audss-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;
@@ -59,6 +65,16 @@ 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)
 {
@@ -68,6 +84,13 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 	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;
+
+	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);
@@ -83,7 +106,10 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	clk_data.clks = clk_table;
-	clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
+	if (variant == TYPE_EXYNOS5420)
+		clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
+	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");
@@ -142,6 +168,12 @@ static int exynos_audss_clk_probe(struct platform_device *pdev)
 				sclk_pcm_p, CLK_SET_RATE_PARENT,
 				reg_base + ASS_CLK_GATE, 5, 0, &lock);
 
+	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);
@@ -188,12 +220,6 @@ static int exynos_audss_clk_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id exynos_audss_clk_of_match[] = {
-	{ .compatible = "samsung,exynos4210-audss-clock", },
-	{ .compatible = "samsung,exynos5250-audss-clock", },
-	{},
-};
-
 static struct platform_driver exynos_audss_clk_driver = {
 	.driver	= {
 		.name = "exynos-audss-clk",
include/dt-bindings/clk/exynos-audss-clk.h +2 -1
index 8279f42..0ae6f5a 100644
@@ -19,7 +19,8 @@
 #define EXYNOS_SCLK_I2S	7
 #define EXYNOS_PCM_BUS		8
 #define EXYNOS_SCLK_PCM	9
+#define EXYNOS_ADMA		10
 
-#define EXYNOS_AUDSS_MAX_CLKS	10
+#define EXYNOS_AUDSS_MAX_CLKS	11
 
 #endif
Signature · 1 line
1.8.4
#42 lore raw

PATCH v4 6/6 ARM: dts: exynos5420: add input clocks to audss clock controller

In reply to Andrew Bresticker

Specify the remaining input clocks (pll_ref, pll_in, and sclk_pcm_in) for the AudioSS clock controller.

---
Changes since v1:
- specified additional input clocks

---
 arch/arm/boot/dts/exynos5420.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/exynos5420.dtsi +2 -2
index d537cd7..056b55e 100644
@@ -72,8 +72,8 @@
 		compatible = "samsung,exynos5420-audss-clock";
 		reg = <0x03810000 0x0C>;
 		#clock-cells = <1>;
-		clocks = <&clock 148>;
-		clock-names = "sclk_audio";
+		clocks = <&clock 1>, <&clock 5>, <&clock 148>, <&clock 149>;
+		clock-names = "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in";
 	};
 
 	codec at 11000000 {
Signature · 1 line
1.8.4
#43 lore raw
In reply to Andrew Bresticker

Hi Mike and Kukjin,

Any decisions regarding this patchset? I believe all comments have been addressed.

Thanks,
Andrew

Quoted 168 lines · Andrew Bresticker

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.

---
Changes since v3:
- __init -> __exit for module exit function
- fixed nits from Sylwester
Changes since v2:
- add error handling to probe callback
- fixed ordering of of_clk_{add,del}_provider
- fixed nits from Tomasz and Sylwester
Changes since v1:
- add clk_unregister() calls to remove callback
- fixed minor nits from Tomasz

---
 drivers/clk/samsung/clk-exynos-audss.c | 104 ++++++++++++++++++++++++++++-----
 1 file changed, 88 insertions(+), 16 deletions(-)
drivers/clk/samsung/clk-exynos-audss.c +88 -17
index 39b40aa..742dabc 100644
@@ -14,6 +14,8 @@
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include <dt-bindings/clk/exynos-audss-clk.h>
 
@@ -62,24 +64,26 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 #endif /* CONFIG_PM_SLEEP */
 
 /* register exynos_audss clocks */
-static void __init exynos_audss_clk_init(struct device_node *np)
+static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
-       reg_base = of_iomap(np, 0);
-       if (!reg_base) {
-               pr_err("%s: failed to map audss registers\n", __func__);
-               return;
+       int i, ret = 0;
+       struct resource *res;
+
+       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 = kzalloc(sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
+       clk_table = devm_kzalloc(&pdev->dev,
+                               sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
                                GFP_KERNEL);
-       if (!clk_table) {
-               pr_err("%s: could not allocate clk lookup table\n", __func__);
-               return;
-       }
+       if (!clk_table)
+               return -ENOMEM;
 
        clk_data.clks = clk_table;
        clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
-       of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 
        clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
                                mout_audss_p, ARRAY_SIZE(mout_audss_p),
@@ -123,13 +127,81 @@ static void __init exynos_audss_clk_init(struct device_node *np)
                                "div_pcm0", CLK_SET_RATE_PARENT,
                                reg_base + ASS_CLK_GATE, 5, 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;
+       }
+
 #ifdef CONFIG_PM_SLEEP
        register_syscore_ops(&exynos_audss_clk_syscore_ops);
 #endif
 
-       pr_info("Exynos: Audss: clock setup completed\n");
+       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;
+}
+
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+       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;
 }
-CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
-               exynos_audss_clk_init);
-CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
-               exynos_audss_clk_init);
+
+static const struct of_device_id exynos_audss_clk_of_match[] = {
+       { .compatible = "samsung,exynos4210-audss-clock", },
+       { .compatible = "samsung,exynos5250-audss-clock", },
+       {},
+};
+
+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)
+{
+       return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);
+
+static void __exit exynos_audss_clk_exit(void)
+{
+       platform_driver_unregister(&exynos_audss_clk_driver);
+}
+module_exit(exynos_audss_clk_exit);
+
+MODULE_AUTHOR("Padmavathi Venna <[email protected]>");
+MODULE_DESCRIPTION("Exynos Audio Subsystem Clock Controller");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:exynos-audss-clk");
--

1.8.4

#44 lore raw

PATCH v4 1/6 clk: exynos-audss: convert to platform device

Hi Mike and Kukjin,

Quoted 7 lines · Andrew Bresticker

Hi Mike and Kukjin,

Any decisions regarding this patchset? I believe all comments have been addressed.

Thanks,
Andrew

Any decisions on this patchset. These can be applied directly on linux-samsung tree.

Quoted 12 lines
Quoted 9 lines · Andrew Bresticker

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.

Quoted 15 lines
Quoted 15 lines

---
Changes since v3:
- __init -> __exit for module exit function
- fixed nits from Sylwester
Changes since v2:
- add error handling to probe callback
- fixed ordering of of_clk_{add,del}_provider
- fixed nits from Tomasz and Sylwester
Changes since v1:
- add clk_unregister() calls to remove callback
- fixed minor nits from Tomasz
---

Thanks
Padma

#45 lore raw

PATCH v4 1/6 clk: exynos-audss: convert to platform device

Quoting Padma Venkat (2013-11-25 22:29:44)

Quoted 6 lines

Hi Mike and Kukjin,

Quoted 2 lines · Andrew Bresticker

Hi Mike and Kukjin,

Quoted 8 lines
Quoted 5 lines

Any decisions regarding this patchset? I believe all comments have been addressed.

Thanks,
Andrew

Any decisions on this patchset. These can be applied directly on linux-samsung tree.

For all of the clk parts:

I'm happy to take them in as part of a pull request.

Regards,
Mike

Quoted 31 lines
Quoted 13 lines
Quoted 9 lines · Andrew Bresticker

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.

Quoted 15 lines
Quoted 15 lines

---
Changes since v3:
- __init -> __exit for module exit function
- fixed nits from Sylwester
Changes since v2:
- add error handling to probe callback
- fixed ordering of of_clk_{add,del}_provider
- fixed nits from Tomasz and Sylwester
Changes since v1:
- add clk_unregister() calls to remove callback
- fixed minor nits from Tomasz
---

Thanks
Padma

#46 lore raw

PATCH v4 1/6 clk: exynos-audss: convert to platform device

Quoted 13 lines · Mike Turquette

Quoting Padma Venkat (2013-11-25 22:29:44)

Quoted 12 lines

Hi Mike and Kukjin,

Quoted 7 lines · Andrew Bresticker

Hi Mike and Kukjin,

Any decisions regarding this patchset? I believe all comments have been addressed.

Thanks,
Andrew

Quoted 9 lines
Quoted 2 lines

Any decisions on this patchset. These can be applied directly on linux-samsung tree.

For all of the clk parts:

I'm happy to take them in as part of a pull request.

OK, Mike, I will take this series into samsung tree and let me send a pull request for your clk tree to avoid useless merge conflicts.

Thanks,
Kukjin

Quoted 34 lines

Regards,
Mike

Quoted 31 lines
Quoted 13 lines
Quoted 9 lines · Andrew Bresticker

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.

Quoted 15 lines
Quoted 15 lines

---
Changes since v3:
- __init -> __exit for module exit function
- fixed nits from Sylwester
Changes since v2:
- add error handling to probe callback
- fixed ordering of of_clk_{add,del}_provider
- fixed nits from Tomasz and Sylwester
Changes since v1:
- add clk_unregister() calls to remove callback
- fixed minor nits from Tomasz
---

Thanks
Padma

#47 lore raw

PATCH v4 1/6 clk: exynos-audss: convert to platform device

Hi Kukjin, Mike,

Quoted 17 lines · On Monday 02 of December 2013 07:43:42 Kukjin Kim wrote:
Quoted 16 lines · Mike Turquette

Quoting Padma Venkat (2013-11-25 22:29:44)

Quoted 14 lines

Hi Mike and Kukjin,

Quoted 7 lines · Andrew Bresticker

Hi Mike and Kukjin,

Any decisions regarding this patchset? I believe all comments have been addressed.

Thanks,
Andrew

Any decisions on this patchset. These can be applied directly on linux-samsung tree.

Quoted 8 lines
Quoted 6 lines

For all of the clk parts:

I'm happy to take them in as part of a pull request.

OK, Mike, I will take this series into samsung tree and let me send a pull request for your clk tree to avoid useless merge conflicts.

Hmm, I can't find this series in any tree, so I guess it has been lost in action.

Should I take it through samsung-clk tree with your acks?

Best regards,
Tomasz

#48 lore raw
Quoted 30 lines · Tomasz Figa

Hi Kukjin, Mike,

Quoted 25 lines · On Monday 02 of December 2013 07:43:42 Kukjin Kim wrote:
Quoted 13 lines · Mike Turquette

Quoting Padma Venkat (2013-11-25 22:29:44)

Quoted 12 lines

Hi Mike and Kukjin,

Quoted 7 lines · Andrew Bresticker

Hi Mike and Kukjin,

Any decisions regarding this patchset? I believe all comments have been addressed.

Thanks,
Andrew

Quoted 9 lines
Quoted 2 lines

Any decisions on this patchset. These can be applied directly on linux-samsung tree.

For all of the clk parts:

I'm happy to take them in as part of a pull request.

OK, Mike, I will take this series into samsung tree and let me send a pull request for your clk tree to avoid useless merge conflicts.

- some persons in Cc

Quoted 3 lines

Hmm, I can't find this series in any tree, so I guess it has been lost in action.

Oops, sorry about that :-(

Quoted 2 lines

Should I take it through samsung-clk tree with your acks?

Tomasz, yes please take this series with my ack.

Thanks,
Kukjin

Acked-by on v4 1/6
#49 lore raw
In reply to Andrew Bresticker

Quoting Andrew Bresticker (2013-09-25 14:12:47)

Quoted 9 lines

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.

Quoted 3 lines
Quoted 155 lines

---
Changes since v3:
- __init -> __exit for module exit function
- fixed nits from Sylwester
Changes since v2:
- add error handling to probe callback
- fixed ordering of of_clk_{add,del}_provider
- fixed nits from Tomasz and Sylwester
Changes since v1:
- add clk_unregister() calls to remove callback
- fixed minor nits from Tomasz

---
 drivers/clk/samsung/clk-exynos-audss.c | 104 ++++++++++++++++++++++++++++-----
 1 file changed, 88 insertions(+), 16 deletions(-)
drivers/clk/samsung/clk-exynos-audss.c +88 -16
index 39b40aa..742dabc 100644
@@ -14,6 +14,8 @@
 #include <linux/clk-provider.h>
 #include <linux/of_address.h>
 #include <linux/syscore_ops.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
 
 #include <dt-bindings/clk/exynos-audss-clk.h>
 
@@ -62,24 +64,26 @@ static struct syscore_ops exynos_audss_clk_syscore_ops = {
 #endif /* CONFIG_PM_SLEEP */
 
 /* register exynos_audss clocks */
-static void __init exynos_audss_clk_init(struct device_node *np)
+static int exynos_audss_clk_probe(struct platform_device *pdev)
 {
-       reg_base = of_iomap(np, 0);
-       if (!reg_base) {
-               pr_err("%s: failed to map audss registers\n", __func__);
-               return;
+       int i, ret = 0;
+       struct resource *res;
+
+       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 = kzalloc(sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
+       clk_table = devm_kzalloc(&pdev->dev,
+                               sizeof(struct clk *) * EXYNOS_AUDSS_MAX_CLKS,
                                GFP_KERNEL);
-       if (!clk_table) {
-               pr_err("%s: could not allocate clk lookup table\n", __func__);
-               return;
-       }
+       if (!clk_table)
+               return -ENOMEM;
 
        clk_data.clks = clk_table;
        clk_data.clk_num = EXYNOS_AUDSS_MAX_CLKS;
-       of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 
        clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
                                mout_audss_p, ARRAY_SIZE(mout_audss_p),
@@ -123,13 +127,81 @@ static void __init exynos_audss_clk_init(struct device_node *np)
                                "div_pcm0", CLK_SET_RATE_PARENT,
                                reg_base + ASS_CLK_GATE, 5, 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;
+       }
+
 #ifdef CONFIG_PM_SLEEP
        register_syscore_ops(&exynos_audss_clk_syscore_ops);
 #endif
 
-       pr_info("Exynos: Audss: clock setup completed\n");
+       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;
+}
+
+static int exynos_audss_clk_remove(struct platform_device *pdev)
+{
+       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;
 }
-CLK_OF_DECLARE(exynos4210_audss_clk, "samsung,exynos4210-audss-clock",
-               exynos_audss_clk_init);
-CLK_OF_DECLARE(exynos5250_audss_clk, "samsung,exynos5250-audss-clock",
-               exynos_audss_clk_init);
+
+static const struct of_device_id exynos_audss_clk_of_match[] = {
+       { .compatible = "samsung,exynos4210-audss-clock", },
+       { .compatible = "samsung,exynos5250-audss-clock", },
+       {},
+};
+
+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)
+{
+       return platform_driver_register(&exynos_audss_clk_driver);
+}
+core_initcall(exynos_audss_clk_init);
+
+static void __exit exynos_audss_clk_exit(void)
+{
+       platform_driver_unregister(&exynos_audss_clk_driver);
+}
+module_exit(exynos_audss_clk_exit);
+
+MODULE_AUTHOR("Padmavathi Venna <[email protected]>");
+MODULE_DESCRIPTION("Exynos Audio Subsystem Clock Controller");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:exynos-audss-clk");
Signature · 1 line
1.8.4