[PATCH/RFC 00/14] ASoC: samsung: Add clk provider for I2S internal clocks

[PATCH/RFC 00/14] ASoC: samsung: Add clk provider for I2S internal clocks

[Sylwester Nawrocki]

This series is an attempt to resolve the CDCLK clock gating issue on Odroid
X2/U3 reported by Daniel Drake [1], by exposing the CDCLK gate clock through
clk API.  The remaining clocks (mux and divider) are also exposed, so the
clk API could be already used instead of the set_sysclk() calls in the machine
drivers.

I need some more thought about interaction between the clk API calls on the
clocks being exposed and the ASoC calls into sound/soc/samsung/i2s.c.
I'm sending teh patches for review though to avoid any waste of time should
it turn out the direction taken is wrong.

This whole series definitely needs more testing, so far I only tested it
on Odroid X2, with the I2S working in slave mode.

[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-September/081753.html

Sylwester Nawrocki (14):
  ASoC: samsung: i2s: Remove unused gpios field from struct i2s
  ASoC: samsung: i2s: samsung_i2s_get_driver_data() cleanup
  ASoC: samsung: i2s: Add return value checks in probe()
  ASoC: samsung: i2s: Request memory region in driver probe()
  ASoC: samsung: i2s: Move clk_get() to platform driver probe()
  ASoC: samsung: i2s: Move clk enable to the platform driver probe()
  ASoC: samsung: i2s: Add get_other_dai helper function
  ASoC: samsung: i2s: Remove an unneeded goto usage
  ASoC: samsung: i2s: Add spinlock in place of local_irq_* calls
  ASoC: samsung: i2s: Protect access to more registers with a spinlock
  ASoC: samsung: odroidx2: Handle I2S CDCLK clock conditionally
  ASoC: samsung: i2s: Add clock provider for the I2S internal clocks
  ARM: dts: Exynos4 and Odroid X2/U3 sound device nodes update
  ARM: dts: Switch Odroid X2/U2 to simple-audio-card

 .../devicetree/bindings/sound/samsung-i2s.txt      |   18 +-
 arch/arm/boot/dts/exynos4.dtsi                     |    9 +
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |   26 +-
 arch/arm/boot/dts/exynos4412-odroidu3.dts          |    8 +-
 arch/arm/boot/dts/exynos4412-odroidx2.dts          |    8 +-
 sound/soc/samsung/i2s.c                            |  365 ++++++++++++--------
 sound/soc/samsung/odroidx2_max98090.c              |    6 +-
 7 files changed, 282 insertions(+), 158 deletions(-)

--
1.7.9.5

[PATCH/RFC 01/14] ASoC: samsung: i2s: Remove unused gpios field from struct i2s

[Sylwester Nawrocki]

The 'gpios' field in 'struct i2s' is now unused, this change
seems to be missing in commit 0429ffeff460c4302bd1520e6
("ASoC: samsung: Remove obsolete GPIO based DT pinmuxing").

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index c1f70fe..2c07592 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -95,7 +95,6 @@ struct i2s_dai {
 	u32	suspend_i2smod;
 	u32	suspend_i2scon;
 	u32	suspend_i2spsr;
-	unsigned long gpios[7];	/* i2s gpio line numbers */
 	const struct samsung_i2s_variant_regs *variant_regs;
 };
 
-- 
1.7.9.5

[PATCH/RFC 02/14] ASoC: samsung: i2s: samsung_i2s_get_driver_data() cleanup

[Sylwester Nawrocki]

Tidy up the samsung_i2s_get_driver_data() function by using
IS_ENABLE() instead of #ifdef and add missing braces for
the 'else' part. Also ensure we are not dereferencing NULL
'match' pointer.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 2c07592..45f7f0f 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -1123,15 +1123,14 @@ static const struct of_device_id exynos_i2s_match[];
 static inline const struct samsung_i2s_dai_data *samsung_i2s_get_driver_data(
 						struct platform_device *pdev)
 {
-#ifdef CONFIG_OF
-	if (pdev->dev.of_node) {
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
 		const struct of_device_id *match;
 		match = of_match_node(exynos_i2s_match, pdev->dev.of_node);
-		return match->data;
-	} else
-#endif
+		return match ? match->data : NULL;
+	} else {
 		return (struct samsung_i2s_dai_data *)
 				platform_get_device_id(pdev)->driver_data;
+	}
 }
 
 #ifdef CONFIG_PM_RUNTIME
-- 
1.7.9.5

[PATCH/RFC 03/14] ASoC: samsung: i2s: Add return value checks in probe()

[Sylwester Nawrocki]

Those function may fail so let's report properly any errors.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 45f7f0f..d65e092 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -1173,11 +1173,13 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "Unable to get drvdata\n");
 			return -EFAULT;
 		}
-		devm_snd_soc_register_component(&sec_dai->pdev->dev,
+		ret = devm_snd_soc_register_component(&sec_dai->pdev->dev,
 						&samsung_i2s_component,
 						&sec_dai->i2s_dai_drv, 1);
-		samsung_asoc_dma_platform_register(&pdev->dev);
-		return 0;
+		if (ret != 0)
+			return ret;
+
+		return samsung_asoc_dma_platform_register(&pdev->dev);
 	}
 
 	pri_dai = i2s_alloc_dai(pdev, false);
@@ -1290,7 +1292,9 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(&pdev->dev);
 
-	samsung_asoc_dma_platform_register(&pdev->dev);
+	ret = samsung_asoc_dma_platform_register(&pdev->dev);
+	if (ret != 0)
+		return ret;
 
 	return 0;
 err:
-- 
1.7.9.5

[PATCH/RFC 04/14] ASoC: samsung: i2s: Request memory region in driver probe()

[Sylwester Nawrocki]

The memory mapped registers region is common for both DAIs so request
it in the I2S platform device driver's probe for the platform device
corresponding to the primary DAI, rather than in the ASoC DAI's probe
callback. While at it switch to devm_ioremap_resource(). This also
drops the hard coded (0x100) register region size in the driver.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |   45 +++++++--------------------------------------
 1 file changed, 7 insertions(+), 38 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index d65e092..5d8aebd 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -59,10 +59,8 @@ struct samsung_i2s_dai_data {
 struct i2s_dai {
 	/* Platform device for this DAI */
 	struct platform_device *pdev;
-	/* IOREMAP'd SFRs */
+	/* Memory mapped SFR region */
 	void __iomem	*addr;
-	/* Physical base address of SFRs */
-	u32	base;
 	/* Rate of RCLK source clock */
 	unsigned long rclk_srcrate;
 	/* Frame Clock */
@@ -979,16 +977,9 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 		goto probe_exit;
 	}
 
-	i2s->addr = ioremap(i2s->base, 0x100);
-	if (i2s->addr == NULL) {
-		dev_err(&i2s->pdev->dev, "cannot ioremap registers\n");
-		return -ENXIO;
-	}
-
 	i2s->clk = clk_get(&i2s->pdev->dev, "iis");
 	if (IS_ERR(i2s->clk)) {
 		dev_err(&i2s->pdev->dev, "failed to get i2s_clock\n");
-		iounmap(i2s->addr);
 		return PTR_ERR(i2s->clk);
 	}
 
@@ -1001,7 +992,6 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 	samsung_asoc_init_dma_data(dai, &i2s->dma_playback, &i2s->dma_capture);
 
 	if (other) {
-		other->addr = i2s->addr;
 		other->clk = i2s->clk;
 	}
 
@@ -1043,8 +1033,6 @@ static int samsung_i2s_dai_remove(struct snd_soc_dai *dai)
 
 		clk_disable_unprepare(i2s->clk);
 		clk_put(i2s->clk);
-
-		iounmap(i2s->addr);
 	}
 
 	i2s->clk = NULL;
@@ -1162,7 +1150,6 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 	u32 regs_base, quirks = 0, idma_addr = 0;
 	struct device_node *np = pdev->dev.of_node;
 	const struct samsung_i2s_dai_data *i2s_dai_data;
-	int ret = 0;
 
 	/* Call during Seconday interface registration */
 	i2s_dai_data = samsung_i2s_get_driver_data(pdev);
@@ -1229,16 +1216,10 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "Unable to get I2S SFR address\n");
-		return -ENXIO;
-	}
+	pri_dai->addr = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pri_dai->addr))
+		return PTR_ERR(pri_dai->addr);
 
-	if (!request_mem_region(res->start, resource_size(res),
-							"samsung-i2s")) {
-		dev_err(&pdev->dev, "Unable to request SFR region\n");
-		return -EBUSY;
-	}
 	regs_base = res->start;
 
 	pri_dai->dma_playback.dma_addr = regs_base + I2STXD;
@@ -1247,7 +1228,6 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 	pri_dai->dma_capture.ch_name = "rx";
 	pri_dai->dma_playback.dma_size = 4;
 	pri_dai->dma_capture.dma_size = 4;
-	pri_dai->base = regs_base;
 	pri_dai->quirks = quirks;
 	pri_dai->variant_regs = i2s_dai_data->i2s_variant_regs;
 
@@ -1258,8 +1238,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 		sec_dai = i2s_alloc_dai(pdev, true);
 		if (!sec_dai) {
 			dev_err(&pdev->dev, "Unable to alloc I2S_sec\n");
-			ret = -ENOMEM;
-			goto err;
+			return -ENOMEM;
 		}
 
 		sec_dai->variant_regs = pri_dai->variant_regs;
@@ -1273,7 +1252,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 		}
 
 		sec_dai->dma_playback.dma_size = 4;
-		sec_dai->base = regs_base;
+		sec_dai->addr = pri_dai->addr;
 		sec_dai->quirks = quirks;
 		sec_dai->idma_playback.dma_addr = idma_addr;
 		sec_dai->pri_dai = pri_dai;
@@ -1282,8 +1261,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 
 	if (i2s_pdata && i2s_pdata->cfg_gpio && i2s_pdata->cfg_gpio(pdev)) {
 		dev_err(&pdev->dev, "Unable to configure gpio\n");
-		ret = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
 
 	devm_snd_soc_register_component(&pri_dai->pdev->dev,
@@ -1297,17 +1275,11 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 		return ret;
 
 	return 0;
-err:
-	if (res)
-		release_mem_region(regs_base, resource_size(res));
-
-	return ret;
 }
 
 static int samsung_i2s_remove(struct platform_device *pdev)
 {
 	struct i2s_dai *i2s, *other;
-	struct resource *res;
 
 	i2s = dev_get_drvdata(&pdev->dev);
 	other = i2s->pri_dai ? : i2s->sec_dai;
@@ -1317,9 +1289,6 @@ static int samsung_i2s_remove(struct platform_device *pdev)
 		other->sec_dai = NULL;
 	} else {
 		pm_runtime_disable(&pdev->dev);
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-		if (res)
-			release_mem_region(res->start, resource_size(res));
 	}
 
 	i2s->pri_dai = NULL;
-- 
1.7.9.5

Re: [PATCH/RFC 04/14] ASoC: samsung: i2s: Request memory region in driver probe()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

On Thu, Dec 11, 2014 at 06:45:42PM +0100, Sylwester Nawrocki wrote:
> The memory mapped registers region is common for both DAIs so request
> it in the I2S platform device driver's probe for the platform device
> corresponding to the primary DAI, rather than in the ASoC DAI's probe
> callback. While at it switch to devm_ioremap_resource(). This also
> drops the hard coded (0x100) register region size in the driver.

This doesn't apply against current code, please check and resend.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH/RFC 04/14] ASoC: samsung: i2s: Request memory region in driver probe()

[Sylwester Nawrocki]

On 12/12/14 19:49, Mark Brown wrote:
> On Thu, Dec 11, 2014 at 06:45:42PM +0100, Sylwester Nawrocki wrote:
>> > The memory mapped registers region is common for both DAIs so request
>> > it in the I2S platform device driver's probe for the platform device
>> > corresponding to the primary DAI, rather than in the ASoC DAI's probe
>> > callback. While at it switch to devm_ioremap_resource(). This also
>> > drops the hard coded (0x100) register region size in the driver.
>
> This doesn't apply against current code, please check and resend.

It seems I've used your topic/samsung branch as a base. Let me rebase
to the for-next branch and resend.

-- 
Regards,
Sylwester

[PATCH/RFC 05/14] ASoC: samsung: i2s: Move clk_get() to platform driver probe()

[Sylwester Nawrocki]

Acquire the I2S interface clock in driver probe() callback
as it's a per-device not a per-DAI clock. While at it switch
to the resource managed clk_get().

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |   19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 5d8aebd..56fe01d 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -971,18 +971,12 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
 	int ret;
 
-	if (other && other->clk) { /* If this is probe on secondary */
+	if (is_secondary(i2s)) { /* If this is probe on the secondary DAI */
 		samsung_asoc_init_dma_data(dai, &other->sec_dai->dma_playback,
 					   NULL);
 		goto probe_exit;
 	}
 
-	i2s->clk = clk_get(&i2s->pdev->dev, "iis");
-	if (IS_ERR(i2s->clk)) {
-		dev_err(&i2s->pdev->dev, "failed to get i2s_clock\n");
-		return PTR_ERR(i2s->clk);
-	}
-
 	ret = clk_prepare_enable(i2s->clk);
 	if (ret != 0) {
 		dev_err(&i2s->pdev->dev, "failed to enable clock: %d\n", ret);
@@ -991,10 +985,6 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 
 	samsung_asoc_init_dma_data(dai, &i2s->dma_playback, &i2s->dma_capture);
 
-	if (other) {
-		other->clk = i2s->clk;
-	}
-
 	if (i2s->quirks & QUIRK_NEED_RSTCLR)
 		writel(CON_RSTCLR, i2s->addr + I2SCON);
 
@@ -1032,7 +1022,6 @@ static int samsung_i2s_dai_remove(struct snd_soc_dai *dai)
 			writel(0, i2s->addr + I2SCON);
 
 		clk_disable_unprepare(i2s->clk);
-		clk_put(i2s->clk);
 	}
 
 	i2s->clk = NULL;
@@ -1222,6 +1211,11 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 
 	regs_base = res->start;
 
+	pri_dai->clk = devm_clk_get(&pdev->dev, "iis");
+	if (IS_ERR(pri_dai->clk)) {
+		dev_err(&pdev->dev, "Failed to get iis clock\n");
+		return PTR_ERR(pri_dai->clk);
+	}
 	pri_dai->dma_playback.dma_addr = regs_base + I2STXD;
 	pri_dai->dma_capture.dma_addr = regs_base + I2SRXD;
 	pri_dai->dma_playback.ch_name = "tx";
@@ -1253,6 +1247,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 
 		sec_dai->dma_playback.dma_size = 4;
 		sec_dai->addr = pri_dai->addr;
+		sec_dai->clk = pri_dai->clk;
 		sec_dai->quirks = quirks;
 		sec_dai->idma_playback.dma_addr = idma_addr;
 		sec_dai->pri_dai = pri_dai;
-- 
1.7.9.5

[PATCH/RFC 06/14] ASoC: samsung: i2s: Move clk enable to the platform driver probe()

[Sylwester Nawrocki]

Gating the I2S bus clock in the driver's runtime PM callbacks has
currently really no effect since the clock is being enabled
in the DAI's probe() and thus is permanently turned on. Now we just
move the enable to the platform driver's probe(), which doesn't
change the situation much. It will allow us to register later on
a clock provider already in samsung_i2s_probe().

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |   25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 56fe01d..871925b 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -969,7 +969,6 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 {
 	struct i2s_dai *i2s = to_info(dai);
 	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
-	int ret;
 
 	if (is_secondary(i2s)) { /* If this is probe on the secondary DAI */
 		samsung_asoc_init_dma_data(dai, &other->sec_dai->dma_playback,
@@ -977,12 +976,6 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 		goto probe_exit;
 	}
 
-	ret = clk_prepare_enable(i2s->clk);
-	if (ret != 0) {
-		dev_err(&i2s->pdev->dev, "failed to enable clock: %d\n", ret);
-		return ret;
-	}
-
 	samsung_asoc_init_dma_data(dai, &i2s->dma_playback, &i2s->dma_capture);
 
 	if (i2s->quirks & QUIRK_NEED_RSTCLR)
@@ -1014,18 +1007,12 @@ probe_exit:
 static int samsung_i2s_dai_remove(struct snd_soc_dai *dai)
 {
 	struct i2s_dai *i2s = snd_soc_dai_get_drvdata(dai);
-	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
-
-	if (!other || !other->clk) {
 
+	if (!is_secondary(i2s)) {
 		if (i2s->quirks & QUIRK_NEED_RSTCLR)
 			writel(0, i2s->addr + I2SCON);
-
-		clk_disable_unprepare(i2s->clk);
 	}
 
-	i2s->clk = NULL;
-
 	return 0;
 }
 
@@ -1139,6 +1126,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 	u32 regs_base, quirks = 0, idma_addr = 0;
 	struct device_node *np = pdev->dev.of_node;
 	const struct samsung_i2s_dai_data *i2s_dai_data;
+	int ret;
 
 	/* Call during Seconday interface registration */
 	i2s_dai_data = samsung_i2s_get_driver_data(pdev);
@@ -1216,6 +1204,12 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "Failed to get iis clock\n");
 		return PTR_ERR(pri_dai->clk);
 	}
+
+	ret = clk_prepare_enable(pri_dai->clk);
+	if (ret != 0) {
+		dev_err(&pdev->dev, "failed to enable clock: %d\n", ret);
+		return ret;
+	}
 	pri_dai->dma_playback.dma_addr = regs_base + I2STXD;
 	pri_dai->dma_capture.dma_addr = regs_base + I2SRXD;
 	pri_dai->dma_playback.ch_name = "tx";
@@ -1286,6 +1280,9 @@ static int samsung_i2s_remove(struct platform_device *pdev)
 		pm_runtime_disable(&pdev->dev);
 	}
 
+	if (!is_secondary(i2s))
+		clk_disable_unprepare(i2s->clk);
+
 	i2s->pri_dai = NULL;
 	i2s->sec_dai = NULL;
 
-- 
1.7.9.5

[PATCH/RFC 07/14] ASoC: samsung: i2s: Add get_other_dai helper function

[Sylwester Nawrocki]

The code to get pointer to the other DAI is repeated multiple
times. Add a helper function and use it to simplify the code
a little.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |   24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 871925b..6501c40 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -130,10 +130,16 @@ static inline bool tx_active(struct i2s_dai *i2s)
 	return active ? true : false;
 }
 
+/* Return pointer to the other DAI */
+static inline struct i2s_dai *get_other_dai(struct i2s_dai *i2s)
+{
+	return i2s->pri_dai ? : i2s->sec_dai;
+}
+
 /* If the other interface of the controller is transmitting data */
 static inline bool other_tx_active(struct i2s_dai *i2s)
 {
-	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
+	struct i2s_dai *other = get_other_dai(i2s);
 
 	return tx_active(other);
 }
@@ -160,7 +166,7 @@ static inline bool rx_active(struct i2s_dai *i2s)
 /* If the other interface of the controller is receiving data */
 static inline bool other_rx_active(struct i2s_dai *i2s)
 {
-	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
+	struct i2s_dai *other = get_other_dai(i2s);
 
 	return rx_active(other);
 }
@@ -461,7 +467,7 @@ static int i2s_set_sysclk(struct snd_soc_dai *dai,
 	  int clk_id, unsigned int rfs, int dir)
 {
 	struct i2s_dai *i2s = to_info(dai);
-	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
+	struct i2s_dai *other = get_other_dai(i2s);
 	u32 mod = readl(i2s->addr + I2SMOD);
 	const struct samsung_i2s_variant_regs *i2s_regs = i2s->variant_regs;
 	unsigned int cdcon_mask = 1 << i2s_regs->cdclkcon_off;
@@ -733,7 +739,7 @@ static int i2s_startup(struct snd_pcm_substream *substream,
 	  struct snd_soc_dai *dai)
 {
 	struct i2s_dai *i2s = to_info(dai);
-	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
+	struct i2s_dai *other = get_other_dai(i2s);
 	unsigned long flags;
 
 	spin_lock_irqsave(&lock, flags);
@@ -760,7 +766,7 @@ static void i2s_shutdown(struct snd_pcm_substream *substream,
 	struct snd_soc_dai *dai)
 {
 	struct i2s_dai *i2s = to_info(dai);
-	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
+	struct i2s_dai *other = get_other_dai(i2s);
 	unsigned long flags;
 	const struct samsung_i2s_variant_regs *i2s_regs = i2s->variant_regs;
 
@@ -791,7 +797,7 @@ static void i2s_shutdown(struct snd_pcm_substream *substream,
 
 static int config_setup(struct i2s_dai *i2s)
 {
-	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
+	struct i2s_dai *other = get_other_dai(i2s);
 	unsigned rfs, bfs, blc;
 	u32 psr;
 
@@ -899,7 +905,7 @@ static int i2s_set_clkdiv(struct snd_soc_dai *dai,
 	int div_id, int div)
 {
 	struct i2s_dai *i2s = to_info(dai);
-	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
+	struct i2s_dai *other = get_other_dai(i2s);
 
 	switch (div_id) {
 	case SAMSUNG_I2S_DIV_BCLK:
@@ -968,7 +974,7 @@ static int i2s_resume(struct snd_soc_dai *dai)
 static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 {
 	struct i2s_dai *i2s = to_info(dai);
-	struct i2s_dai *other = i2s->pri_dai ? : i2s->sec_dai;
+	struct i2s_dai *other = get_other_dai(i2s);
 
 	if (is_secondary(i2s)) { /* If this is probe on the secondary DAI */
 		samsung_asoc_init_dma_data(dai, &other->sec_dai->dma_playback,
@@ -1271,7 +1277,7 @@ static int samsung_i2s_remove(struct platform_device *pdev)
 	struct i2s_dai *i2s, *other;
 
 	i2s = dev_get_drvdata(&pdev->dev);
-	other = i2s->pri_dai ? : i2s->sec_dai;
+	other = get_other_dai(i2s);
 
 	if (other) {
 		other->pri_dai = NULL;
-- 
1.7.9.5

[PATCH/RFC 08/14] ASoC: samsung: i2s: Remove an unneeded goto usage

[Sylwester Nawrocki]

The usage of this goto seems unjustified, use if/else statement instead.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |   17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 6501c40..a131621 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -979,19 +979,18 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 	if (is_secondary(i2s)) { /* If this is probe on the secondary DAI */
 		samsung_asoc_init_dma_data(dai, &other->sec_dai->dma_playback,
 					   NULL);
-		goto probe_exit;
-	}
-
-	samsung_asoc_init_dma_data(dai, &i2s->dma_playback, &i2s->dma_capture);
+	} else {
+		samsung_asoc_init_dma_data(dai, &i2s->dma_playback,
+					   &i2s->dma_capture);
 
-	if (i2s->quirks & QUIRK_NEED_RSTCLR)
-		writel(CON_RSTCLR, i2s->addr + I2SCON);
+		if (i2s->quirks & QUIRK_NEED_RSTCLR)
+			writel(CON_RSTCLR, i2s->addr + I2SCON);
 
-	if (i2s->quirks & QUIRK_SUPPORTS_IDMA)
-		idma_reg_addr_init(i2s->addr,
+		if (i2s->quirks & QUIRK_SUPPORTS_IDMA)
+			idma_reg_addr_init(i2s->addr,
 					i2s->sec_dai->idma_playback.dma_addr);
+	}
 
-probe_exit:
 	/* Reset any constraint on RFS and BFS */
 	i2s->rfs = 0;
 	i2s->bfs = 0;
-- 
1.7.9.5

[PATCH/RFC 09/14] ASoC: samsung: i2s: Add spinlock in place of local_irq_* calls

[Sylwester Nawrocki]

It seems this driver hasn't been updated for SMP, as local_irq_save/
local_irq_restore doesn't provide proper protection of read/modify/write
of the device's registers on such systems. Introduce a spinlock serializing
access to the register region, it will be helpful later when some of
the registers are made also accessible through the clk API.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index a131621..e8e0107 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -94,6 +94,10 @@ struct i2s_dai {
 	u32	suspend_i2scon;
 	u32	suspend_i2spsr;
 	const struct samsung_i2s_variant_regs *variant_regs;
+
+	/* Spinlock protecting access to the device's registers */
+	spinlock_t spinlock;
+	spinlock_t *lock;
 };
 
 /* Lock for cross i/f checks */
@@ -867,10 +871,10 @@ static int i2s_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		local_irq_save(flags);
+		spin_lock_irqsave(i2s->lock, flags);
 
 		if (config_setup(i2s)) {
-			local_irq_restore(flags);
+			spin_unlock_irqrestore(i2s->lock, flags);
 			return -EINVAL;
 		}
 
@@ -879,12 +883,12 @@ static int i2s_trigger(struct snd_pcm_substream *substream,
 		else
 			i2s_txctrl(i2s, 1);
 
-		local_irq_restore(flags);
+		spin_unlock_irqrestore(i2s->lock, flags);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		local_irq_save(flags);
+		spin_lock_irqsave(i2s->lock, flags);
 
 		if (capture) {
 			i2s_rxctrl(i2s, 0);
@@ -894,7 +898,7 @@ static int i2s_trigger(struct snd_pcm_substream *substream,
 			i2s_fifo(i2s, FIC_TXFLUSH);
 		}
 
-		local_irq_restore(flags);
+		spin_unlock_irqrestore(i2s->lock, flags);
 		break;
 	}
 
@@ -1157,6 +1161,9 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	spin_lock_init(&pri_dai->spinlock);
+	pri_dai->lock = &pri_dai->spinlock;
+
 	if (!np) {
 		res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 		if (!res) {
@@ -1234,6 +1241,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 			return -ENOMEM;
 		}
 
+		sec_dai->lock = &pri_dai->spinlock;
 		sec_dai->variant_regs = pri_dai->variant_regs;
 		sec_dai->dma_playback.dma_addr = regs_base + I2STXDS;
 		sec_dai->dma_playback.ch_name = "tx-sec";
-- 
1.7.9.5

[PATCH/RFC 10/14] ASoC: samsung: i2s: Protect access to more registers with a spinlock

[Sylwester Nawrocki]

It seems this driver hasn't been updated for SMP, as local_irq_save/
local_irq_restore doesn't provide proper protection of read/modify/
write of the device's registers on such systems. Introduce a spinlock
serializing access to the registers, it will be helpful later when
I2SMOD, I2SPSR registers are made also accessible through the clk API.

Possibly some changes within this patch are not required, I'm not
confident what exactly needs to be serialized in the driver and what
is already taken care for in the ASoC core. At least I tried to cover
the I2SMOD, I2SPSR registers. Since the I2S register region is common
for 2 DAIs I suspect the serialization here might need again a careful
review.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/i2s.c |   81 +++++++++++++++++++++++++++++------------------
 1 file changed, 51 insertions(+), 30 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index e8e0107..825138b 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -472,17 +472,22 @@ static int i2s_set_sysclk(struct snd_soc_dai *dai,
 {
 	struct i2s_dai *i2s = to_info(dai);
 	struct i2s_dai *other = get_other_dai(i2s);
-	u32 mod = readl(i2s->addr + I2SMOD);
 	const struct samsung_i2s_variant_regs *i2s_regs = i2s->variant_regs;
 	unsigned int cdcon_mask = 1 << i2s_regs->cdclkcon_off;
 	unsigned int rsrc_mask = 1 << i2s_regs->rclksrc_off;
+	u32 mod, mask, val = 0;
+
+	spin_lock(i2s->lock);
+	mod = readl(i2s->addr + I2SMOD);
+	spin_unlock(i2s->lock);
 
 	switch (clk_id) {
 	case SAMSUNG_I2S_OPCLK:
-		mod &= ~MOD_OPCLK_MASK;
-		mod |= dir;
+		mask = MOD_OPCLK_MASK;
+		val = dir;
 		break;
 	case SAMSUNG_I2S_CDCLK:
+		mask = 1 << i2s_regs->cdclkcon_off;
 		/* Shouldn't matter in GATING(CLOCK_IN) mode */
 		if (dir == SND_SOC_CLOCK_IN)
 			rfs = 0;
@@ -499,15 +504,15 @@ static int i2s_set_sysclk(struct snd_soc_dai *dai,
 		}
 
 		if (dir == SND_SOC_CLOCK_IN)
-			mod |= 1 << i2s_regs->cdclkcon_off;
-		else
-			mod &= ~(1 << i2s_regs->cdclkcon_off);
+			val = 1 << i2s_regs->cdclkcon_off;
 
 		i2s->rfs = rfs;
 		break;
 
 	case SAMSUNG_I2S_RCLKSRC_0: /* clock corrsponding to IISMOD[10] := 0 */
 	case SAMSUNG_I2S_RCLKSRC_1: /* clock corrsponding to IISMOD[10] := 1 */
+		mask = 1 << i2s_regs->rclksrc_off;
+
 		if ((i2s->quirks & QUIRK_NO_MUXPSR)
 				|| (clk_id == SAMSUNG_I2S_RCLKSRC_0))
 			clk_id = 0;
@@ -557,18 +562,19 @@ static int i2s_set_sysclk(struct snd_soc_dai *dai,
 			return 0;
 		}
 
-		if (clk_id == 0)
-			mod &= ~(1 << i2s_regs->rclksrc_off);
-		else
-			mod |= 1 << i2s_regs->rclksrc_off;
-
+		if (clk_id == 1)
+			val = 1 << i2s_regs->rclksrc_off;
 		break;
 	default:
 		dev_err(&i2s->pdev->dev, "We don't serve that!\n");
 		return -EINVAL;
 	}
 
+	spin_lock(i2s->lock);
+	mod = readl(i2s->addr + I2SMOD);
+	mod = (mod & ~mask) | val;
 	writel(mod, i2s->addr + I2SMOD);
+	spin_unlock(i2s->lock);
 
 	return 0;
 }
@@ -577,9 +583,8 @@ static int i2s_set_fmt(struct snd_soc_dai *dai,
 	unsigned int fmt)
 {
 	struct i2s_dai *i2s = to_info(dai);
-	u32 mod = readl(i2s->addr + I2SMOD);
 	int lrp_shift, sdf_shift, sdf_mask, lrp_rlow, mod_slave;
-	u32 tmp = 0;
+	u32 mod, tmp = 0;
 
 	lrp_shift = i2s->variant_regs->lrp_off;
 	sdf_shift = i2s->variant_regs->sdf_off;
@@ -639,12 +644,15 @@ static int i2s_set_fmt(struct snd_soc_dai *dai,
 		return -EINVAL;
 	}
 
+	spin_lock(i2s->lock);
+	mod = readl(i2s->addr + I2SMOD);
 	/*
 	 * Don't change the I2S mode if any controller is active on this
 	 * channel.
 	 */
 	if (any_active(i2s) &&
 		((mod & (sdf_mask | lrp_rlow | mod_slave)) != tmp)) {
+		spin_unlock(i2s->lock);
 		dev_err(&i2s->pdev->dev,
 				"%s:%d Other DAI busy\n", __func__, __LINE__);
 		return -EAGAIN;
@@ -653,6 +661,7 @@ static int i2s_set_fmt(struct snd_soc_dai *dai,
 	mod &= ~(sdf_mask | lrp_rlow | mod_slave);
 	mod |= tmp;
 	writel(mod, i2s->addr + I2SMOD);
+	spin_unlock(i2s->lock);
 
 	return 0;
 }
@@ -661,16 +670,16 @@ static int i2s_hw_params(struct snd_pcm_substream *substream,
 	struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
 {
 	struct i2s_dai *i2s = to_info(dai);
-	u32 mod = readl(i2s->addr + I2SMOD);
+	u32 mod, mask = 0, val = 0;
 
 	if (!is_secondary(i2s))
-		mod &= ~(MOD_DC2_EN | MOD_DC1_EN);
+		mask |= (MOD_DC2_EN | MOD_DC1_EN);
 
 	switch (params_channels(params)) {
 	case 6:
-		mod |= MOD_DC2_EN;
+		val |= MOD_DC2_EN;
 	case 4:
-		mod |= MOD_DC1_EN;
+		val |= MOD_DC1_EN;
 		break;
 	case 2:
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
@@ -692,44 +701,49 @@ static int i2s_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	if (is_secondary(i2s))
-		mod &= ~MOD_BLCS_MASK;
+		mask |= MOD_BLCS_MASK;
 	else
-		mod &= ~MOD_BLCP_MASK;
+		mask |= MOD_BLCP_MASK;
 
 	if (is_manager(i2s))
-		mod &= ~MOD_BLC_MASK;
+		mask |= MOD_BLC_MASK;
 
 	switch (params_width(params)) {
 	case 8:
 		if (is_secondary(i2s))
-			mod |= MOD_BLCS_8BIT;
+			val |= MOD_BLCS_8BIT;
 		else
-			mod |= MOD_BLCP_8BIT;
+			val |= MOD_BLCP_8BIT;
 		if (is_manager(i2s))
-			mod |= MOD_BLC_8BIT;
+			val |= MOD_BLC_8BIT;
 		break;
 	case 16:
 		if (is_secondary(i2s))
-			mod |= MOD_BLCS_16BIT;
+			val |= MOD_BLCS_16BIT;
 		else
-			mod |= MOD_BLCP_16BIT;
+			val |= MOD_BLCP_16BIT;
 		if (is_manager(i2s))
-			mod |= MOD_BLC_16BIT;
+			val |= MOD_BLC_16BIT;
 		break;
 	case 24:
 		if (is_secondary(i2s))
-			mod |= MOD_BLCS_24BIT;
+			val |= MOD_BLCS_24BIT;
 		else
-			mod |= MOD_BLCP_24BIT;
+			val |= MOD_BLCP_24BIT;
 		if (is_manager(i2s))
-			mod |= MOD_BLC_24BIT;
+			val |= MOD_BLC_24BIT;
 		break;
 	default:
 		dev_err(&i2s->pdev->dev, "Format(%d) not supported\n",
 				params_format(params));
 		return -EINVAL;
 	}
+
+	spin_lock(i2s->lock);
+	mod = readl(i2s->addr + I2SMOD);
+	mod = (mod & ~mask) | val;
 	writel(mod, i2s->addr + I2SMOD);
+	spin_unlock(i2s->lock);
 
 	samsung_asoc_init_dma_data(dai, &i2s->dma_playback, &i2s->dma_capture);
 
@@ -979,6 +993,7 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 {
 	struct i2s_dai *i2s = to_info(dai);
 	struct i2s_dai *other = get_other_dai(i2s);
+	unsigned long flags;
 
 	if (is_secondary(i2s)) { /* If this is probe on the secondary DAI */
 		samsung_asoc_init_dma_data(dai, &other->sec_dai->dma_playback,
@@ -999,11 +1014,14 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai)
 	i2s->rfs = 0;
 	i2s->bfs = 0;
 	i2s->rclk_srcrate = 0;
+
+	spin_lock_irqsave(i2s->lock, flags);
 	i2s_txctrl(i2s, 0);
 	i2s_rxctrl(i2s, 0);
 	i2s_fifo(i2s, FIC_TXFLUSH);
 	i2s_fifo(other, FIC_TXFLUSH);
 	i2s_fifo(i2s, FIC_RXFLUSH);
+	spin_unlock_irqrestore(i2s->lock, flags);
 
 	/* Gate CDCLK by default */
 	if (!is_opened(other))
@@ -1018,8 +1036,11 @@ static int samsung_i2s_dai_remove(struct snd_soc_dai *dai)
 	struct i2s_dai *i2s = snd_soc_dai_get_drvdata(dai);
 
 	if (!is_secondary(i2s)) {
-		if (i2s->quirks & QUIRK_NEED_RSTCLR)
+		if (i2s->quirks & QUIRK_NEED_RSTCLR) {
+			spin_lock(i2s->lock);
 			writel(0, i2s->addr + I2SCON);
+			spin_unlock(i2s->lock);
+		}
 	}
 
 	return 0;
-- 
1.7.9.5

[PATCH/RFC 11/14] ASoC: samsung: odroidx2: Handle I2S CDCLK clock conditionally

[Sylwester Nawrocki]

If the codec control it's master clock provider by the I2S module
we should not be touching it with set_sysclk() calls.
So skip the set_sysclk() call in the machine driver if "clocks"
property is found in the codec device node.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 sound/soc/samsung/odroidx2_max98090.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/soc/samsung/odroidx2_max98090.c b/sound/soc/samsung/odroidx2_max98090.c
index 3c8f604..ca94938 100644
--- a/sound/soc/samsung/odroidx2_max98090.c
+++ b/sound/soc/samsung/odroidx2_max98090.c
@@ -21,6 +21,8 @@ struct odroidx2_drv_data {
 /* The I2S CDCLK output clock frequency for the MAX98090 codec */
 #define MAX98090_MCLK 19200000
 
+static struct snd_soc_dai_link odroidx2_dai[];
+
 static int odroidx2_late_probe(struct snd_soc_card *card)
 {
 	struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai;
@@ -29,7 +31,9 @@ static int odroidx2_late_probe(struct snd_soc_card *card)
 
 	ret = snd_soc_dai_set_sysclk(codec_dai, 0, MAX98090_MCLK,
 						SND_SOC_CLOCK_IN);
-	if (ret < 0)
+
+	if (ret < 0 || of_find_property(odroidx2_dai[0].codec_of_node,
+					"clocks", NULL))
 		return ret;
 
 	/* Set the cpu DAI configuration in order to use CDCLK */
-- 
1.7.9.5

Re: [PATCH/RFC 11/14] ASoC: samsung: odroidx2: Handle I2S CDCLK clock conditionally

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 510 bytes --]

On Thu, Dec 11, 2014 at 06:45:49PM +0100, Sylwester Nawrocki wrote:
> If the codec control it's master clock provider by the I2S module
> we should not be touching it with set_sysclk() calls.
> So skip the set_sysclk() call in the machine driver if "clocks"
> property is found in the codec device node.

Please clarify in the changelog here that this is a transition measure
to support old DTs, it's not clear why we might want to do things for
the I2S controller by fishing around in the CODEC DT otherwise.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH/RFC 11/14] ASoC: samsung: odroidx2: Handle I2S CDCLK clock conditionally

[Sylwester Nawrocki]

On 12/12/14 19:53, Mark Brown wrote:
> On Thu, Dec 11, 2014 at 06:45:49PM +0100, Sylwester Nawrocki wrote:
>> > If the codec control it's master clock provider by the I2S module
>> > we should not be touching it with set_sysclk() calls.
>> > So skip the set_sysclk() call in the machine driver if "clocks"
>> > property is found in the codec device node.
>
> Please clarify in the changelog here that this is a transition measure
> to support old DTs, it's not clear why we might want to do things for
> the I2S controller by fishing around in the CODEC DT otherwise.

Indeed, there is some room for improvement. I'll add such note to the
commit description. Presumably this whole driver could be removed
after a kernel release or two, since we switched Odroid X2/U3 to
the simple-card.

-- 
Regards,
Sylwester

[PATCH/RFC 12/14] ASoC: samsung: i2s: Add clock provider for the I2S internal clocks

[Sylwester Nawrocki]

This patch adds clock provider (currently only for DT platforms) for
the CODECLKO (CDCLK) gate, RCLKSRC mux and RCLK pre-scaler divider
divider clock. Those all tree clock are only available in the IIS
Multi Audio Interface (I2S0), the regular IIS Bus Interface has only
CDCLK gate clock.

The motivation behind this patch is to expose the I2S internal clocks
which are currently controlled through set_sysclk() through the clk
API, so dedicated sound machine driver per each board can be avoided.

The intention is also to fix the CDCLK gating issue reported by
Daniel Drake:
http://mailman.alsa-project.org/pipermail/alsa-devel/2014-September/081753.html

This patch also reverts commit b97c60abf9a561f86ae71bd741add02673cc1
("ASoC: samsung-i2s: Maintain CDCLK settings across i2s_{shutdown/
startup}") The problem that commit attempted to solve only affects
the Odroid X2/U3, which doesn't configure the CDCLK clock in
struct snd_soc_dai_ops hw_params callback and the issue should be
now resolved by using clk API, i.e. having the codec enabling/
disabling the CDCLK clock as required.

Cc: devicetree@vger.kernel.org
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 .../devicetree/bindings/sound/samsung-i2s.txt      |   18 ++-
 sound/soc/samsung/i2s.c                            |  116 ++++++++++++++++----
 2 files changed, 113 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
index d188296..fb3b777 100644
--- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
@@ -34,12 +34,26 @@ Required SoC Specific Properties:
   clk. i2s0 has internal mux to select the source of root clk and i2s1 and i2s2
   doesn't have any such mux.

-Optional SoC Specific Properties:
+Optional Properties:

 - samsung,idma-addr: Internal DMA register base address of the audio
   sub system(used in secondary sound source).
 - pinctrl-0: Should specify pin control groups used for this controller.
 - pinctrl-names: Should contain only one value - "default".
+- #clock-cells: should be 1, this property must be present if the I2S device
+  is a clock provider in terms of the common clock bindings, described in
+  ../clock/clock-bindings.txt.
+- clock-output-names: from the common clock bindings, names of the CDCLK
+  I2S output clocks, suggested values are "i2s_cdclk0", "i2s_cdclk1",
+  "i2s_cdclk3" for the I2S0, I2S1, I2S2 devices recpectively.
+
+The assignment of indices for the I2Sx clock provider is as follows:
+ 0 - the CDCLK (CODECLKO) gate clock,
+ 1 - the RCLK prescaler divider clock (corresponding to the IISPSR register),
+ 2 - the RCLKSRC mux clock (corresponding to RCLKSRC bit in register IISMOD).
+
+Clocks 1, 2 are normally only available in the IIS Mutli Audio Interface (I2S0).
+

 Example:

@@ -54,6 +68,8 @@ i2s0: i2s@03830000 {
 		<&clock_audss EXYNOS_I2S_BUS>,
 		<&clock_audss EXYNOS_SCLK_I2S>;
 	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
+	#clock-cells;
+	clock-output-names = "i2s_cdclk0";
 	samsung,idma-addr = <0x03000000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2s0_bus>;
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 825138b..3f4d2c0 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -13,6 +13,7 @@
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -81,8 +82,6 @@ struct i2s_dai {
 #define DAI_OPENED	(1 << 0) /* Dai is opened */
 #define DAI_MANAGER	(1 << 1) /* Dai is the manager */
 	unsigned mode;
-	/* CDCLK pin direction: 0  - input, 1 - output */
-	unsigned int cdclk_out:1;
 	/* Driver for this DAI */
 	struct snd_soc_dai_driver i2s_dai_drv;
 	/* DMA parameters */
@@ -98,6 +97,10 @@ struct i2s_dai {
 	/* Spinlock protecting access to the device's registers */
 	spinlock_t spinlock;
 	spinlock_t *lock;
+
+	/* Below fields are only valid if this is the primary FIFO */
+	struct clk *clk_table[3];
+	struct clk_onecell_data clk_data;
 };

 /* Lock for cross i/f checks */
@@ -774,9 +777,6 @@ static int i2s_startup(struct snd_pcm_substream *substream,

 	spin_unlock_irqrestore(&lock, flags);

-	if (!is_opened(other) && i2s->cdclk_out)
-		i2s_set_sysclk(dai, SAMSUNG_I2S_CDCLK,
-				0, SND_SOC_CLOCK_OUT);
 	return 0;
 }

@@ -786,31 +786,20 @@ static void i2s_shutdown(struct snd_pcm_substream *substream,
 	struct i2s_dai *i2s = to_info(dai);
 	struct i2s_dai *other = get_other_dai(i2s);
 	unsigned long flags;
-	const struct samsung_i2s_variant_regs *i2s_regs = i2s->variant_regs;

 	spin_lock_irqsave(&lock, flags);

 	i2s->mode &= ~DAI_OPENED;
 	i2s->mode &= ~DAI_MANAGER;

-	if (is_opened(other)) {
+	if (is_opened(other))
 		other->mode |= DAI_MANAGER;
-	} else {
-		u32 mod = readl(i2s->addr + I2SMOD);
-		i2s->cdclk_out = !(mod & (1 << i2s_regs->cdclkcon_off));
-		if (other)
-			other->cdclk_out = i2s->cdclk_out;
-	}
+
 	/* Reset any constraint on RFS and BFS */
 	i2s->rfs = 0;
 	i2s->bfs = 0;

 	spin_unlock_irqrestore(&lock, flags);
-
-	/* Gate CDCLK by default */
-	if (!is_opened(other))
-		i2s_set_sysclk(dai, SAMSUNG_I2S_CDCLK,
-				0, SND_SOC_CLOCK_IN);
 }

 static int config_setup(struct i2s_dai *i2s)
@@ -1147,6 +1136,91 @@ static int i2s_runtime_resume(struct device *dev)
 }
 #endif /* CONFIG_PM_RUNTIME */

+static void i2s_unregister_clocks(struct i2s_dai *i2s)
+{
+	int i;
+
+	for (i = 0; i < i2s->clk_data.clk_num; i++) {
+		if (!IS_ERR(i2s->clk_table[i]))
+			clk_unregister(i2s->clk_table[i]);
+	}
+}
+
+static void i2s_unregister_clock_provider(struct platform_device *pdev)
+{
+	struct i2s_dai *i2s = dev_get_drvdata(&pdev->dev);
+
+	of_clk_del_provider(pdev->dev.of_node);
+	i2s_unregister_clocks(i2s);
+}
+
+static int i2s_register_clock_provider(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct i2s_dai *i2s = dev_get_drvdata(dev);
+	const char *clk_name[2] = { "i2s_opclk0", "i2s_opclk1" };
+	const char *p_names[2] = { NULL };
+	const struct samsung_i2s_variant_regs *reg_info = i2s->variant_regs;
+	struct clk *rclksrc;
+	int ret, i;
+
+	/* Register the clock provider only if it's expected in the DTB */
+	if (!of_find_property(dev->of_node, "#clock-cells", NULL))
+		return 0;
+
+	/* Get the RCLKSRC mux clock parent clock names */
+	for (i = 0; i < ARRAY_SIZE(p_names); i++) {
+		rclksrc = clk_get(dev, clk_name[i]);
+		if (IS_ERR(rclksrc))
+			continue;
+		p_names[i] = __clk_get_name(rclksrc);
+		clk_put(rclksrc);
+	}
+	/*
+	 * Register the mux and div clocks only if both source clocks
+	 * are available, i.e. for the I2S0 instance and versions with
+	 * QUIRK_NO_MUXPSR quirk not set.
+	 */
+	if (p_names[1] && !(i2s->quirks & QUIRK_NO_MUXPSR)) {
+		/* Activate the prescaler */
+		u32 val = readl(i2s->addr + I2SPSR);
+		writel(val | PSR_PSREN, i2s->addr + I2SPSR);
+
+		i2s->clk_table[1] = clk_register_mux(NULL, "i2s_rclksrc",
+				p_names, ARRAY_SIZE(p_names),
+				CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
+				i2s->addr + I2SMOD, reg_info->rclksrc_off,
+				1, 0, i2s->lock);
+
+		i2s->clk_table[2] = clk_register_divider(NULL,
+				"i2s_presc", "i2s_rclksrc",
+				CLK_SET_RATE_PARENT,
+				i2s->addr + I2SPSR, 8, 6, 0, i2s->lock);
+
+		p_names[0] = "i2s_presc";
+		i2s->clk_data.clk_num = 2;
+	}
+	of_property_read_string_index(dev->of_node,
+				"clock-output-names", 0, &clk_name[0]);
+
+	i2s->clk_table[0] = clk_register_gate(NULL, clk_name[0],
+				p_names[0], CLK_SET_RATE_PARENT,
+				i2s->addr + I2SMOD, reg_info->cdclkcon_off,
+				CLK_GATE_SET_TO_DISABLE, i2s->lock);
+
+	i2s->clk_data.clk_num += 1;
+	i2s->clk_data.clks = i2s->clk_table;
+
+	ret = of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
+				  &i2s->clk_data);
+	if (ret < 0) {
+		dev_err(dev, "failed to add clock provider\n");
+		i2s_unregister_clocks(i2s);
+	}
+
+	return ret;
+}
+
 static int samsung_i2s_probe(struct platform_device *pdev)
 {
 	struct i2s_dai *pri_dai, *sec_dai = NULL;
@@ -1297,7 +1371,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 	if (ret != 0)
 		return ret;

-	return 0;
+	return i2s_register_clock_provider(pdev);
 }

 static int samsung_i2s_remove(struct platform_device *pdev)
@@ -1314,8 +1388,10 @@ static int samsung_i2s_remove(struct platform_device *pdev)
 		pm_runtime_disable(&pdev->dev);
 	}

-	if (!is_secondary(i2s))
+	if (!is_secondary(i2s)) {
+		i2s_unregister_clock_provider(pdev);
 		clk_disable_unprepare(i2s->clk);
+	}

 	i2s->pri_dai = NULL;
 	i2s->sec_dai = NULL;
--
1.7.9.5

Re: [alsa-devel] [PATCH/RFC 12/14] ASoC: samsung: i2s: Add clock provider for the I2S internal clocks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

On Thu, Dec 11, 2014 at 06:45:50PM +0100, Sylwester Nawrocki wrote:

> +Optional Properties:
> 
>  - samsung,idma-addr: Internal DMA register base address of the audio
>    sub system(used in secondary sound source).
>  - pinctrl-0: Should specify pin control groups used for this controller.
>  - pinctrl-names: Should contain only one value - "default".
> +- #clock-cells: should be 1, this property must be present if the I2S device
> +  is a clock provider in terms of the common clock bindings, described in
> +  ../clock/clock-bindings.txt.
> +- clock-output-names: from the common clock bindings, names of the CDCLK
> +  I2S output clocks, suggested values are "i2s_cdclk0", "i2s_cdclk1",
> +  "i2s_cdclk3" for the I2S0, I2S1, I2S2 devices recpectively.
> +

Why not make this mandatory going forwards?  You can add a note saying
that older DTs may not have it.

> +The assignment of indices for the I2Sx clock provider is as follows:
> + 0 - the CDCLK (CODECLKO) gate clock,
> + 1 - the RCLK prescaler divider clock (corresponding to the IISPSR register),
> + 2 - the RCLKSRC mux clock (corresponding to RCLKSRC bit in register IISMOD).

Why use the indicies here, they only seem to add obscurity?

> +		clk_put(rclksrc);
> +	}
> +	/*

Missing blank line.

> +	 * Register the mux and div clocks only if both source clocks
> +	 * are available, i.e. for the I2S0 instance and versions with
> +	 * QUIRK_NO_MUXPSR quirk not set.
> +	 */

Why not proceed even if one is missing?  This repeats in words the if
statement but doesn't explain why we're doing this.

> +	ret = of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
> +				  &i2s->clk_data);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to add clock provider\n");

Best practice is to print the error code.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [alsa-devel] [PATCH/RFC 12/14] ASoC: samsung: i2s: Add clock provider for the I2S internal clocks

[Sylwester Nawrocki]

On 12/12/14 21:03, Mark Brown wrote:
> On Thu, Dec 11, 2014 at 06:45:50PM +0100, Sylwester Nawrocki wrote:
> 
>> +Optional Properties:
>>
>>  - samsung,idma-addr: Internal DMA register base address of the audio
>>    sub system(used in secondary sound source).
>>  - pinctrl-0: Should specify pin control groups used for this controller.
>>  - pinctrl-names: Should contain only one value - "default".
>> +- #clock-cells: should be 1, this property must be present if the I2S device
>> +  is a clock provider in terms of the common clock bindings, described in
>> +  ../clock/clock-bindings.txt.
>> +- clock-output-names: from the common clock bindings, names of the CDCLK
>> +  I2S output clocks, suggested values are "i2s_cdclk0", "i2s_cdclk1",
>> +  "i2s_cdclk3" for the I2S0, I2S1, I2S2 devices recpectively.
>> +
> 
> Why not make this mandatory going forwards?  You can add a note saying
> that older DTs may not have it.

I guess that makes more sense, I'll move it to the mandatory properties
section.

>> +The assignment of indices for the I2Sx clock provider is as follows:
>> + 0 - the CDCLK (CODECLKO) gate clock,
>> + 1 - the RCLK prescaler divider clock (corresponding to the IISPSR register),
>> + 2 - the RCLKSRC mux clock (corresponding to RCLKSRC bit in register IISMOD).
> 
> Why use the indicies here, they only seem to add obscurity?

I didn't want to create a separate header file, with all the GPL copyright
notice header etc, for just this enumeration.
Anyway I'll put these as macro definitions into include/dt-bindings/sound
/samsung-i2s.h.

>> +		clk_put(rclksrc);
>> +	}
>> +	/*
> 
> Missing blank line.
> 
>> +	 * Register the mux and div clocks only if both source clocks
>> +	 * are available, i.e. for the I2S0 instance and versions with
>> +	 * QUIRK_NO_MUXPSR quirk not set.
>> +	 */
> 
> Why not proceed even if one is missing?  This repeats in words the if
> statement but doesn't explain why we're doing this.

Right, I should have said here that this is really to distinguish between
I2S0 and I2S1, I2S2. So we register the mux and div clocks only for
the IIS Multi Audio Interface (I2S0) and not for the IIS Bus Interface
(I2S1, I2S2) instances.
However, it seems I got confused by same compatible string set for
I2S0..I2S2 in arch/arm/boot/dts/exynos4.dtsi - "samsung,s5pv210-i2s",
where for I2S1, I2S2 it should be "samsung,s3c6410-i2s". So we have,
for example, QUIRK_NO_MUXPSR set for them. There seems to be no similar
bug in arch/arm/boot/dts/exynos5250.dtsi.
I'll drop that mux input clock test the and fix the compatible strings
for exynos4 in dts.

>> +	ret = of_clk_add_provider(dev->of_node, of_clk_src_onecell_get,
>> +				  &i2s->clk_data);
>> +	if (ret < 0) {
>> +		dev_err(dev, "failed to add clock provider\n");
> 
> Best practice is to print the error code.

Yeah, I'll add it.

--
Regards,
Sylwester

[PATCH/RFC 13/14] ARM: dts: Exynos4 and Odroid X2/U3 sound device nodes update

[Sylwester Nawrocki]

Clock related properties are added to the Exynos4 I2S device nodes
so they can be referred to as clock providers. Missing i2s_opclk1
clock is added to the I2S0 node and clock properties are added
to the MAX98090 codec node to allow it to control/read frequency
of the MCLK clock directly.

Cc: devicetree@vger.kernel.org
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi                  |    6 ++++++
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi |    7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index e0278ec..e1ce457 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -61,6 +61,8 @@
 		reg = <0x03830000 0x100>;
 		clocks = <&clock_audss EXYNOS_I2S_BUS>;
 		clock-names = "iis";
+		#clock-cells = <1>;
+		clock-output-names = "i2s_cdclk0";
 		dmas = <&pdma0 12>, <&pdma0 11>, <&pdma0 10>;
 		dma-names = "tx", "rx", "tx-sec";
 		samsung,idma-addr = <0x03000000>;
@@ -372,6 +374,8 @@
 		reg = <0x13960000 0x100>;
 		clocks = <&clock CLK_I2S1>;
 		clock-names = "iis";
+		#clock-cells = <1>;
+		clock-output-names = "i2s_cdclk1";
 		dmas = <&pdma1 12>, <&pdma1 11>;
 		dma-names = "tx", "rx";
 		status = "disabled";
@@ -382,6 +386,8 @@
 		reg = <0x13970000 0x100>;
 		clocks = <&clock CLK_I2S2>;
 		clock-names = "iis";
+		#clock-cells = <1>;
+		clock-output-names = "i2s_cdclk2";
 		dmas = <&pdma0 14>, <&pdma0 13>;
 		dma-names = "tx", "rx";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index b41950f..956737e 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -37,8 +37,9 @@
 		pinctrl-names = "default";
 		status = "okay";
 		clocks = <&clock_audss EXYNOS_I2S_BUS>,
-			 <&clock_audss EXYNOS_DOUT_AUD_BUS>;
-		clock-names = "iis", "i2s_opclk0";
+			 <&clock_audss EXYNOS_DOUT_AUD_BUS>,
+			 <&clock_audss EXYNOS_SCLK_I2S>;
+		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
 	};

 	sound: sound {
@@ -357,6 +358,8 @@
 			reg = <0x10>;
 			interrupt-parent = <&gpx0>;
 			interrupts = <0 0>;
+			clocks = <&i2s0 0>;
+			clock-names = "mclk";
 		};
 	};

--
1.7.9.5

[PATCH/RFC 14/14] ARM: dts: Switch Odroid X2/U2 to simple-audio-card

[Sylwester Nawrocki]

Now when the CDCLK I2S output clock can be handled through the clock
API the Odroid X2/U3 can be switched to the simple-audio-card DT binding.

Cc: devicetree@vger.kernel.org
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi                  |    3 +++
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi |   19 ++++++++++++++++---
 arch/arm/boot/dts/exynos4412-odroidu3.dts       |    8 +++++---
 arch/arm/boot/dts/exynos4412-odroidx2.dts       |    8 ++++++--
 4 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index e1ce457..5f9e72f 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -66,6 +66,7 @@
 		dmas = <&pdma0 12>, <&pdma0 11>, <&pdma0 10>;
 		dma-names = "tx", "rx", "tx-sec";
 		samsung,idma-addr = <0x03000000>;
+		#sound-dai-cells = <1>;
 		status = "disabled";
 	};

@@ -378,6 +379,7 @@
 		clock-output-names = "i2s_cdclk1";
 		dmas = <&pdma1 12>, <&pdma1 11>;
 		dma-names = "tx", "rx";
+		#sound-dai-cells = <1>;
 		status = "disabled";
 	};

@@ -390,6 +392,7 @@
 		clock-output-names = "i2s_cdclk2";
 		dmas = <&pdma0 14>, <&pdma0 13>;
 		dma-names = "tx", "rx";
+		#sound-dai-cells = <1>;
 		status = "disabled";
 	};

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 956737e..89fed7a 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -43,9 +43,7 @@
 	};

 	sound: sound {
-		compatible = "samsung,odroidx2-audio";
-		samsung,i2s-controller = <&i2s0>;
-		samsung,audio-codec = <&max98090>;
+		compatible = "simple-audio-card";
 		assigned-clocks = <&clock_audss EXYNOS_MOUT_AUDSS>,
 				<&clock_audss EXYNOS_MOUT_I2S>,
 				<&clock_audss EXYNOS_DOUT_SRP>,
@@ -56,6 +54,20 @@
 				<0>,
 				<192000000>,
 				<19200000>;
+
+		simple-audio-card,format = "i2s";
+		simple-audio-card,bitclock-master = <&link0_codec>;
+		simple-audio-card,frame-master = <&link0_codec>;
+
+		simple-audio-card,cpu {
+			sound-dai = <&i2s0 0>;
+			system-clock-frequency = <19200000>;
+		};
+
+		link0_codec: simple-audio-card,codec {
+			sound-dai = <&max98090>;
+			clocks = <&i2s0 0>;
+		};
 	};

 	mmc@12550000 {
@@ -360,6 +372,7 @@
 			interrupts = <0 0>;
 			clocks = <&i2s0 0>;
 			clock-names = "mclk";
+			#sound-dai-cells = <0>;
 		};
 	};

diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index c8a64be..44684e5 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -49,9 +49,11 @@
 };

 &sound {
-	compatible = "samsung,odroidu3-audio";
-	samsung,model = "Odroid-U3";
-	samsung,audio-routing =
+	simple-audio-card,name = "Odroid-U3";
+	simple-audio-card,widgets =
+		"Headphone", "Headphone Jack",
+		"Speakers", "Speakers";
+	simple-audio-card,routing =
 		"Headphone Jack", "HPL",
 		"Headphone Jack", "HPR",
 		"Headphone Jack", "MICBIAS",
diff --git a/arch/arm/boot/dts/exynos4412-odroidx2.dts b/arch/arm/boot/dts/exynos4412-odroidx2.dts
index 96b43f4..6e33678 100644
--- a/arch/arm/boot/dts/exynos4412-odroidx2.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidx2.dts
@@ -23,8 +23,12 @@
 };

 &sound {
-	samsung,model = "Odroid-X2";
-	samsung,audio-routing =
+	simple-audio-card,name = "Odroid-X2";
+	simple-audio-card,widgets =
+		"Headphone", "Headphone Jack",
+		"Microphone", "Mic Jack",
+		"Microphone", "DMIC";
+	simple-audio-card,routing =
 		"Headphone Jack", "HPL",
 		"Headphone Jack", "HPR",
 		"IN1", "Mic Jack",
--
1.7.9.5

Re: [alsa-devel] [PATCH/RFC 00/14] ASoC: samsung: Add clk provider for I2S internal clocks

[Padma Venkat]

Hi Sylwester,

> I need some more thought about interaction between the clk API calls on the
> clocks being exposed and the ASoC calls into sound/soc/samsung/i2s.c.
> I'm sending teh patches for review though to avoid any waste of time should
> it turn out the direction taken is wrong.
>
> This whole series definitely needs more testing, so far I only tested it
> on Odroid X2, with the I2S working in slave mode.

your patches looks good to me. I did basic playback and capture test
of your patches with i2s in master mode on exynos7.  I also added
secondary dai for testing and verified playback. But I think some more
testing required to check the serialization of registers. Tried aplay
with front dai in background and with secondary dai in the foreground.
But due to one issue on my system backend playback not working.

Thanks
Padma

>
> [1]
> http://mailman.alsa-project.org/pipermail/alsa-devel/2014-September/081753.html

Re: [alsa-devel] [PATCH/RFC 00/14] ASoC: samsung: Add clk provider for I2S internal clocks

[Sylwester Nawrocki]

Hi Padma,

On 16/12/14 04:16, Padma Venkat wrote:
> Hi Sylwester,
> 
>> I need some more thought about interaction between the clk API calls on the
>> clocks being exposed and the ASoC calls into sound/soc/samsung/i2s.c.
>> I'm sending teh patches for review though to avoid any waste of time should
>> it turn out the direction taken is wrong.
>>
>> This whole series definitely needs more testing, so far I only tested it
>> on Odroid X2, with the I2S working in slave mode.
> 
> your patches looks good to me. I did basic playback and capture test
> of your patches with i2s in master mode on exynos7.  I also added
> secondary dai for testing and verified playback. But I think some more
> testing required to check the serialization of registers. Tried aplay
> with front dai in background and with secondary dai in the foreground.
> But due to one issue on my system backend playback not working.

Thanks a lot for testing! I also tested concurrent playback with the
primary and the secondary DAI (on Exynos4412), enabling the secondary DAI
with a bit hackish patch as in this branch:
git://linuxtv.org/snawrocki/samsung.git sound/odroidx2-simple-card-v2
I'm planning to do necessary changes to enable the secondary DAI properly
with simple-card, once we're done with this series.

-- 
Regards,
Sylwester