[PATCH v2 00/16] ASoC: samsung: Add clk provider for I2S internal clocks

[PATCH v2 00/16] 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 as reported by Daniel Drake [1], by exposing the CDCLK gate clock 
(and the two other clocks) through clk API. 

Changes since previous version:
 - removed check for the i2s_opclk1 mux input clock while creating the mux
   and div clocks,
 - the DT binding documentation changes reworked (addressing review comments),
 - added include/dt-bindings/sound/samsung-i2s.h header file defining
   the clk indices, it's been put into a separate patch together with the I2S 
   DT binding documentation updates to make merging of the ASoC and the dts
   patches separately easier,
 - a patch fixing compatible strings of I2S1, I2S2 in exynos4.dtsi is included
   in this series.

This whole series may need more testing on other SoCs, so far I only tested 
it on Odroid Exynos4412 X2, with the I2S working in slave mode.

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

Sylwester Nawrocki (16):
  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 more registers with a spinlock
  ASoC: samsung: odroidx2: Handle I2S CDCLK clock conditionally
  ASoC: samsung: i2s: Add clk provider DT binding documentation
  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
  ARM: dts: Fix I2S1, I2S2 compatible for exynos4 SoCs

 .../devicetree/bindings/sound/samsung-i2s.txt      |   22 ++
 arch/arm/boot/dts/exynos4.dtsi                     |   13 +-
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |   27 +-
 arch/arm/boot/dts/exynos4412-odroidu3.dts          |    8 +-
 arch/arm/boot/dts/exynos4412-odroidx2.dts          |    8 +-
 include/dt-bindings/sound/samsung-i2s.h            |    8 +
 sound/soc/samsung/i2s.c                            |  362 ++++++++++++--------
 sound/soc/samsung/odroidx2_max98090.c              |    6 +-
 8 files changed, 295 insertions(+), 159 deletions(-)
 create mode 100644 include/dt-bindings/sound/samsung-i2s.h

-- 
1.7.9.5

[PATCH v2 01/16] 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 eca17dc..827e4b4 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

Re: [PATCH v2 01/16] ASoC: samsung: i2s: Remove unused gpios field from struct i2s

[Mark Brown]

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

On Fri, Dec 19, 2014 at 02:55:21PM +0100, Sylwester Nawrocki wrote:
> 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").

Applied, thanks.

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

[PATCH v2 02/16] 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 827e4b4..430298c 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

Re: [PATCH v2 02/16] ASoC: samsung: i2s: samsung_i2s_get_driver_data() cleanup

[Mark Brown]

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

On Fri, Dec 19, 2014 at 02:55:22PM +0100, Sylwester Nawrocki wrote:
> 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.

This doesn't apply against current code, can you please check and
resend?

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

Re: [PATCH v2 02/16] ASoC: samsung: i2s: samsung_i2s_get_driver_data() cleanup

[Sylwester Nawrocki]

On 06/01/15 19:14, Mark Brown wrote:
> On Fri, Dec 19, 2014 at 02:55:22PM +0100, Sylwester Nawrocki wrote:
>> > 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.
>
> This doesn't apply against current code, can you please check and
> resend?

OK, I will resend it rebased onto your 3.19-rc1 topic/samsung branch.

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

[Sylwester Nawrocki]

These functions may fail so let's properly report 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 430298c..8ab6734 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 v2 04/16] 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 8ab6734..6c7f956 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

[PATCH v2 05/16] 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 6c7f956..7f967b2 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 v2 06/16] 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 a clock
provider already in samsung_i2s_probe() function.

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 7f967b2..80b5362 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

Re: [PATCH v2 06/16] ASoC: samsung: i2s: Move clk enable to the platform driver probe()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1107 bytes --]

On Fri, Dec 19, 2014 at 02:55:26PM +0100, Sylwester Nawrocki wrote:
> 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 a clock
> provider already in samsung_i2s_probe() function.

That doesn't sound quite right - the normal idiom for this stuff is to
enable on probe, mark the device as active then if runtime PM is enabled
it can idle the device and turn off the clock.  That way if runtime PM
is disabled things continue to run.  I've not checked to see if this is
actually happening correctly all the way through but that's what's
supposed to happen and means that enabling in the probe should still
result in working clock management.

It will as things currently stand be broken for the dual DAI case so
this is a fix for that (the dual DAI case will double enable if both
links are in use) but the analysis isn't quite correct.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2 06/16] ASoC: samsung: i2s: Move clk enable to the platform driver probe()

[Sylwester Nawrocki]

On 06/01/15 19:06, Mark Brown wrote:
> On Fri, Dec 19, 2014 at 02:55:26PM +0100, Sylwester Nawrocki wrote:
>> 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 a clock
>> provider already in samsung_i2s_probe() function.
> 
> That doesn't sound quite right - the normal idiom for this stuff is to
> enable on probe, mark the device as active then if runtime PM is enabled
> it can idle the device and turn off the clock.  That way if runtime PM
> is disabled things continue to run.  I've not checked to see if this is
> actually happening correctly all the way through but that's what's
> supposed to happen and means that enabling in the probe should still
> result in working clock management.
> 
> It will as things currently stand be broken for the dual DAI case so
> this is a fix for that (the dual DAI case will double enable if both
> links are in use) but the analysis isn't quite correct.

I agree that the device should be additionally marked as active, however
things will stop working unless we also do other changes to ensure
the BUSCLK is enabled when required.

AFAIU the clock will not be double enabled when both links are used,
since the clk_get/clk_prepare_enable part in DAI probe() is done only
for the primary DAI. I actually checked it with single and both links
and the clk_enable_count is always 1 after system initialization:

# grep . /sys/kernel/debug/clk/*i2s*/clk_enable_count

/sys/kernel/debug/clk/dout_i2s/clk_enable_count:0
/sys/kernel/debug/clk/i2s0/clk_enable_count:0
/sys/kernel/debug/clk/i2s1/clk_enable_count:0
/sys/kernel/debug/clk/i2s2/clk_enable_count:0
/sys/kernel/debug/clk/i2s_bus/clk_enable_count:1
/sys/kernel/debug/clk/mout_i2s/clk_enable_count:0
/sys/kernel/debug/clk/sclk_i2s/clk_enable_count:0
/sys/kernel/debug/clk/sclk_i2s1/clk_enable_count:0
/sys/kernel/debug/clk/sclk_i2s2/clk_enable_count:0

I'm going to rephrase the changelog to something like:

"The clk_prepare_enable() call on the "iis" clock is moved to happen
 earlier in the DAI platform device driver's probe() callback, so the
 I2S registers can be safely accessed through the clk API, after the clk
 supplier is registered in the platform device probe().

 After this patch the "iis" clock is kept enabled since the (primary)
 I2S platform device probe() and until the platform device driver remove()
 call.  This is similar to gating the clock in the snd_soc_dai probe() and
 remove() callbacks.
 Normally, in addition to that we should mark the device as PM runtime
 active, so if runtime PM is enabled it can idle the device by turning off
 the clock.  Correcting this issue is left for a separate patch series,
 as we need to ensure the BUSCLK clock is always enabled when required."

[PATCH v2 07/16] 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 instead.

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 80b5362..c4d4ccc 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 v2 08/16] 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 c4d4ccc..cd66e58 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 v2 09/16] 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 don'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 I2SMOD, I2SPSR 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 cd66e58..b5b6b1e 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 v2 10/16] ASoC: samsung: i2s: Protect more registers with a spinlock

[Sylwester Nawrocki]

Ensure the I2SMOD, I2SPSR registers, which are also exposed through
clk API are only accessed with the i2s->spinlock spinlock held.

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 b5b6b1e..49ba623 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 v2 11/16] ASoC: samsung: odroidx2: Handle I2S CDCLK clock conditionally

[Sylwester Nawrocki]

In order to support old DTs we check the codec device node if it
contains "clocks" property and only if it doesn't (which indicates
an old DT) we proceed with enabling the CDCLK clock by means of
the set_sysclk() callback.  For new DTs which use the common clock
bindings for CDCLK that clock is supposed to be handled outside
the sound machine driver.

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 d7640e7..5a87794 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

[PATCH v2 12/16] ASoC: samsung: i2s: Add clk provider DT binding documentation

[Sylwester Nawrocki]

The new DT properties required for the I2S device node to be referred
as a clock provider and corresponding clock indices definition is added.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 .../devicetree/bindings/sound/samsung-i2s.txt      |   22 ++++++++++++++++++++
 include/dt-bindings/sound/samsung-i2s.h            |    8 +++++++
 2 files changed, 30 insertions(+)
 create mode 100644 include/dt-bindings/sound/samsung-i2s.h

diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
index d188296..09e0e18 100644
--- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
@@ -33,6 +33,25 @@ Required SoC Specific Properties:
   "iis" is the i2s bus clock and i2s_opclk0, i2s_opclk1 are sources of the root
   clk. i2s0 has internal mux to select the source of root clk and i2s1 and i2s2
   doesn't have any such mux.
+- #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.
+
+There are following clocks available at the I2S device nodes:
+ CLK_I2S_CDCLK    - the CDCLK (CODECLKO) gate clock,
+ CLK_I2S_RCLK_PSR - the RCLK prescaler divider clock (corresponding to the
+		    IISPSR register),
+ CLK_I2S_RCLK_SRC - the RCLKSRC mux clock (corresponding to RCLKSRC bit in
+		    IISMOD register).
+
+Refer to the SoC datasheet for availability of the above clocks.
+The CLK_I2S_RCLK_PSR and CLK_I2S_RCLK_SRC clocks are usually only available
+in the IIS Multi Audio Interface (I2S0).
+Note: Old DTs may not have the #clock-cells, clock-output-names properties
+and then not use the I2S node as a clock supplier.
 
 Optional SoC Specific Properties:
 
@@ -41,6 +60,7 @@ Optional SoC Specific Properties:
 - pinctrl-0: Should specify pin control groups used for this controller.
 - pinctrl-names: Should contain only one value - "default".
 
+
 Example:
 
 i2s0: i2s@03830000 {
@@ -54,6 +74,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/include/dt-bindings/sound/samsung-i2s.h b/include/dt-bindings/sound/samsung-i2s.h
new file mode 100644
index 0000000..0c69818d
--- /dev/null
+++ b/include/dt-bindings/sound/samsung-i2s.h
@@ -0,0 +1,8 @@
+#ifndef _DT_BINDINGS_SAMSUNG_I2S_H
+#define _DT_BINDINGS_SAMSUNG_I2S_H
+
+#define CLK_I2S_CDCLK		0
+#define CLK_I2S_RCLK_SRC	1
+#define CLK_I2S_RCLK_PSR	2
+
+#endif /* _DT_BINDINGS_SAMSUNG_I2S_H */
-- 
1.7.9.5

[PATCH v2 13/16] ASoC: samsung: i2s: Add clock provider for the I2S internal clocks

[Sylwester Nawrocki]

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

With that in place the simple-card can be used for the Odroid X2/U3,
and likely for more boards which now use a dedicated sound machine
drivers.

This also resolvesthe 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.

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

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 49ba623..8846906 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -10,9 +10,11 @@
  * published by the Free Software Foundation.
  */

+#include <dt-bindings/sound/samsung-i2s.h>
 #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 +83,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 +98,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 +778,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 +787,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 +1137,87 @@ 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);
+	}
+
+	if (!(i2s->quirks & QUIRK_NO_MUXPSR)) {
+		/* Activate the prescaler */
+		u32 val = readl(i2s->addr + I2SPSR);
+		writel(val | PSR_PSREN, i2s->addr + I2SPSR);
+
+		i2s->clk_table[CLK_I2S_RCLK_SRC] = 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[CLK_I2S_RCLK_PSR] = 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[CLK_I2S_CDCLK] = 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: %d\n", ret);
+		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 +1368,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 +1385,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

[PATCH v2 14/16] 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.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi                  |    6 ++++++
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi |    8 ++++++--
 2 files changed, 12 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..9ccdc1f 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -7,6 +7,7 @@
  * published by the Free Software Foundation.
 */
 
+#include <dt-bindings/sound/samsung-i2s.h>
 #include <dt-bindings/input/input.h>
 #include "exynos4412.dtsi"
 
@@ -37,8 +38,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 +359,8 @@
 			reg = <0x10>;
 			interrupt-parent = <&gpx0>;
 			interrupts = <0 0>;
+			clocks = <&i2s0 CLK_I2S_CDCLK>;
+			clock-names = "mclk";
 		};
 	};
 
-- 
1.7.9.5

[PATCH v2 15/16] 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.

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 9ccdc1f..051343e 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -44,9 +44,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>,
@@ -57,6 +55,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 CLK_I2S_CDCLK>;
+		};
 	};
 
 	mmc@12550000 {
@@ -361,6 +373,7 @@
 			interrupts = <0 0>;
 			clocks = <&i2s0 CLK_I2S_CDCLK>;
 			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

[PATCH v2 16/16] ARM: dts: Fix I2S1, I2S2 compatible for exynos4 SoCs

[Sylwester Nawrocki]

I2S1, I2S2 on Exynos4 SoC series have limited functionality compared
to I2S0, "samsung,s3c6410-i2s" compatible should be used for them.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 5f9e72f..71dc876 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -371,7 +371,7 @@
 	};
 
 	i2s1: i2s@13960000 {
-		compatible = "samsung,s5pv210-i2s";
+		compatible = "samsung,s3c6410-i2s";
 		reg = <0x13960000 0x100>;
 		clocks = <&clock CLK_I2S1>;
 		clock-names = "iis";
@@ -384,7 +384,7 @@
 	};
 
 	i2s2: i2s@13970000 {
-		compatible = "samsung,s5pv210-i2s";
+		compatible = "samsung,s3c6410-i2s";
 		reg = <0x13970000 0x100>;
 		clocks = <&clock CLK_I2S2>;
 		clock-names = "iis";
-- 
1.7.9.5

Re: [PATCH v2 16/16] ARM: dts: Fix I2S1, I2S2 compatible for exynos4 SoCs

[Mark Brown]

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

On Fri, Dec 19, 2014 at 02:55:36PM +0100, Sylwester Nawrocki wrote:
> I2S1, I2S2 on Exynos4 SoC series have limited functionality compared
> to I2S0, "samsung,s3c6410-i2s" compatible should be used for them.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---

This should probably go to stable as a bug fix too.

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

Re: [PATCH v2 16/16] ARM: dts: Fix I2S1, I2S2 compatible for exynos4 SoCs

[Sylwester Nawrocki]

On 06/01/15 19:15, Mark Brown wrote:
> On Fri, Dec 19, 2014 at 02:55:36PM +0100, Sylwester Nawrocki wrote:
>> > I2S1, I2S2 on Exynos4 SoC series have limited functionality compared
>> > to I2S0, "samsung,s3c6410-i2s" compatible should be used for them.
>> > 
>> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> > ---
>
> This should probably go to stable as a bug fix too.

Sounds like a reasonable thing to do, I will Cc stable in next iteration.

Re: [PATCH v2 00/16] ASoC: samsung: Add clk provider for I2S internal clocks

[Mark Brown]

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

On Fri, Dec 19, 2014 at 02:55:20PM +0100, Sylwester Nawrocki wrote:
> This series is an attempt to resolve the CDCLK clock gating issue on Odroid
> X2/U3 as reported by Daniel Drake [1], by exposing the CDCLK gate clock 
> (and the two other clocks) through clk API. 

This all looks basically fine apart from the comments I made but since I
couldn't apply patch 2 I've not been able to to test anything.

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

Re: [PATCH v2 00/16] ASoC: samsung: Add clk provider for I2S internal clocks

[Tobias Jakobi]

Hello,

I've tested the series on my X2 and so far I haven't encountered any
obvious issues with it.

I have a small question though. With the move to simple-audio-card the
old driver (selected by SND_SOC_ODROIDX2) is probably going away after
some time.
Currently SND_SOC_ODROIDX2 also selects SND_SOC_MAX98090 and
SND_SAMSUNG_I2S, which I believe (correct me if I'm wrong) are necessary
to use the sound subsystem even with simple-audio-card (since the
max98090 codec has to be built at least). I've found no way of selecting
these two manually, so at the moment I need both CONFIG_SND_SIMPLE_CARD
and SND_SOC_ODROIDX2 enabled to use sound. Is this intended behaviour?


And while I'm at it. I'm booting the board with an upstream u-boot
version which sets the MPLL to 800MHz, which AFAIK is the clocking rate
that makes less 'trouble' (but at the expense of performance).
I was wondering if anyone can comment on whether the recent rework has
any influence on the behaviour when booting with the vendor u-boot
(which sets the MPLL to 880MHz). IIRC then clock rounding issues arose
there.

It would be interesting to know if this series gets us closer to drive
the MPLL with the recommended (?) higher clocking rate.


With best wishes,
Tobias



Sylwester Nawrocki wrote:
> This series is an attempt to resolve the CDCLK clock gating issue on Odroid
> X2/U3 as reported by Daniel Drake [1], by exposing the CDCLK gate clock 
> (and the two other clocks) through clk API. 
> 
> Changes since previous version:
>  - removed check for the i2s_opclk1 mux input clock while creating the mux
>    and div clocks,
>  - the DT binding documentation changes reworked (addressing review comments),
>  - added include/dt-bindings/sound/samsung-i2s.h header file defining
>    the clk indices, it's been put into a separate patch together with the I2S 
>    DT binding documentation updates to make merging of the ASoC and the dts
>    patches separately easier,
>  - a patch fixing compatible strings of I2S1, I2S2 in exynos4.dtsi is included
>    in this series.
> 
> This whole series may need more testing on other SoCs, so far I only tested 
> it on Odroid Exynos4412 X2, with the I2S working in slave mode.
> 
> [1] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-September/081753.html
> 
> Sylwester Nawrocki (16):
>   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 more registers with a spinlock
>   ASoC: samsung: odroidx2: Handle I2S CDCLK clock conditionally
>   ASoC: samsung: i2s: Add clk provider DT binding documentation
>   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
>   ARM: dts: Fix I2S1, I2S2 compatible for exynos4 SoCs
> 
>  .../devicetree/bindings/sound/samsung-i2s.txt      |   22 ++
>  arch/arm/boot/dts/exynos4.dtsi                     |   13 +-
>  arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |   27 +-
>  arch/arm/boot/dts/exynos4412-odroidu3.dts          |    8 +-
>  arch/arm/boot/dts/exynos4412-odroidx2.dts          |    8 +-
>  include/dt-bindings/sound/samsung-i2s.h            |    8 +
>  sound/soc/samsung/i2s.c                            |  362 ++++++++++++--------
>  sound/soc/samsung/odroidx2_max98090.c              |    6 +-
>  8 files changed, 295 insertions(+), 159 deletions(-)
>  create mode 100644 include/dt-bindings/sound/samsung-i2s.h
> 

Re: [PATCH v2 00/16] ASoC: samsung: Add clk provider for I2S internal clocks

[Sylwester Nawrocki]

Hello Tobias,

On 24/01/15 02:53, Tobias Jakobi wrote:
> Hello,
> 
> I've tested the series on my X2 and so far I haven't encountered any
> obvious issues with it.
> 
> I have a small question though. With the move to simple-audio-card the
> old driver (selected by SND_SOC_ODROIDX2) is probably going away after
> some time.
> Currently SND_SOC_ODROIDX2 also selects SND_SOC_MAX98090 and
> SND_SAMSUNG_I2S, which I believe (correct me if I'm wrong) are necessary
> to use the sound subsystem even with simple-audio-card (since the
> max98090 codec has to be built at least). I've found no way of selecting
> these two manually, so at the moment I need both CONFIG_SND_SIMPLE_CARD
> and SND_SOC_ODROIDX2 enabled to use sound. Is this intended behaviour?

Thanks for the feedback.  It seems I finally forgot to update Kconfig,
presumably we should make SND_SOC_ODROIDX2 selecting SND_SIMPLE_CARD,
i.e.

diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig
index 58c797a..7fb728c 100644
--- a/sound/soc/samsung/Kconfig
+++ b/sound/soc/samsung/Kconfig
@@ -247,6 +247,7 @@ config SND_SOC_ODROIDX2
        depends on SND_SOC_SAMSUNG
        select SND_SOC_MAX98090
        select SND_SAMSUNG_I2S
+       select SND_SIMPLE_CARD
        help
          Say Y here to enable audio support for the Odroidx2.

Making SND_SOC_SAMSUNG selecting SND_SIMPLE_CARD is probably not a good
idea and still we would be missing the codec part.
I'm going to post a patch as above.

> And while I'm at it. I'm booting the board with an upstream u-boot
> version which sets the MPLL to 800MHz, which AFAIK is the clocking rate
> that makes less 'trouble' (but at the expense of performance).
> I was wondering if anyone can comment on whether the recent rework has
> any influence on the behaviour when booting with the vendor u-boot
> (which sets the MPLL to 880MHz). IIRC then clock rounding issues arose
> there.
> 
> It would be interesting to know if this series gets us closer to drive
> the MPLL with the recommended (?) higher clocking rate.

No, (unfortunately) this series has nothing to do with MPLL.  The whole
clock tree for sound is configured by the kernel, including the EPLL, which
provides reference clock for the audio subsystem.  MPLL and EPLL are normally
used in different clock domains.  So there should be no any clock rounding
issues related to bootloader as far as audio is concerned.  Please let me
know if you find any related problems.

-- 
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html