All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V4 0/4] Add i2s support on smdk5420
@ 2013-08-12  9:49 ` Padmavathi Venna
  0 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, alsa-devel, devicetree,
	padma.v, padma.kvr
  Cc: broonie, kgene.kim, tomasz.figa, abrestic

Samsung has different versions of I2S introduced in different
platforms. Each version has some new support added for multichannel,
secondary fifo, s/w reset control, internal mux for rclk src clk and
tdm support. Each newly added change has a quirk. So this patch adds
all the required quirks as driver data and based on compatible string
from dtsi fetches the quirks. This also adds i2s support on exynos5420.

Changes since V3:
	- Addressed review comments by Tomasz Figa related to const qualifier
	  for samsung_i2s_dai_data
	- Removed passing quirks as driver data for non-dt platforms. 
	- Separated out adding i2s nodes and enabling audio support on 5420
	  into different patch set as they are dependent on some of already posted
	  but not yet merged i2c, dwmmc, dma and audss clock controller patches.

Changes since V2:
        - Separated out driver side changes and dts changes in two
          patch sets.
        - Replaced samsung,s3c6410-i2s-v4 with samsung,s3c6410-i2s-multi
          for more clarity as suggested by Tomasz Figa.

Changes since V1:
        - Pass quirks as driver data and fetch the quirks based on
          compatible string from dtsi file as suggested by
          Tomasz Figa and Mark Brown
        - Make the I2S driver more flexible with respect to register
          access as suggested by Tomasz Figa and Mark Brown
        - Add 5420 support in the driver.
        - Modify the dtsi files with the corresponding compatible
          strings and removed the i2s quirks from 5250 dtsi file.
        - Updated the i2s Documentation with relevent changes and
          i2s versioning info.
        - Add i2s nodes on exynos5420.dtsi
        - Enable sound support on smdk5420

This patch set is made based on Mark Brown for-next branch on sound.git.

Padmavathi Venna (4):
  ASoC: Samsung: I2S: Add quirks as driver data in I2S
  ASoC: Samsung: I2S: Modify the I2S driver to support I2S on
    Exynos5420
  ARM: dts: exynos5250: move common i2s properties to exynos5 dtsi
  ARM: dts: Change i2s compatible string on exynos5250

 .../devicetree/bindings/sound/samsung-i2s.txt      |   22 ++--
 arch/arm/boot/dts/exynos5.dtsi                     |   21 +++
 arch/arm/boot/dts/exynos5250.dtsi                  |   17 +---
 include/linux/platform_data/asoc-s3c.h             |    1 +
 sound/soc/samsung/i2s-regs.h                       |   15 ++
 sound/soc/samsung/i2s.c                            |  143 +++++++++++++++-----
 6 files changed, 157 insertions(+), 62 deletions(-)

-- 
1.7.4.4

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

* [PATCH V4 0/4] Add i2s support on smdk5420
@ 2013-08-12  9:49 ` Padmavathi Venna
  0 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-arm-kernel

Samsung has different versions of I2S introduced in different
platforms. Each version has some new support added for multichannel,
secondary fifo, s/w reset control, internal mux for rclk src clk and
tdm support. Each newly added change has a quirk. So this patch adds
all the required quirks as driver data and based on compatible string
from dtsi fetches the quirks. This also adds i2s support on exynos5420.

Changes since V3:
	- Addressed review comments by Tomasz Figa related to const qualifier
	  for samsung_i2s_dai_data
	- Removed passing quirks as driver data for non-dt platforms. 
	- Separated out adding i2s nodes and enabling audio support on 5420
	  into different patch set as they are dependent on some of already posted
	  but not yet merged i2c, dwmmc, dma and audss clock controller patches.

Changes since V2:
        - Separated out driver side changes and dts changes in two
          patch sets.
        - Replaced samsung,s3c6410-i2s-v4 with samsung,s3c6410-i2s-multi
          for more clarity as suggested by Tomasz Figa.

Changes since V1:
        - Pass quirks as driver data and fetch the quirks based on
          compatible string from dtsi file as suggested by
          Tomasz Figa and Mark Brown
        - Make the I2S driver more flexible with respect to register
          access as suggested by Tomasz Figa and Mark Brown
        - Add 5420 support in the driver.
        - Modify the dtsi files with the corresponding compatible
          strings and removed the i2s quirks from 5250 dtsi file.
        - Updated the i2s Documentation with relevent changes and
          i2s versioning info.
        - Add i2s nodes on exynos5420.dtsi
        - Enable sound support on smdk5420

This patch set is made based on Mark Brown for-next branch on sound.git.

Padmavathi Venna (4):
  ASoC: Samsung: I2S: Add quirks as driver data in I2S
  ASoC: Samsung: I2S: Modify the I2S driver to support I2S on
    Exynos5420
  ARM: dts: exynos5250: move common i2s properties to exynos5 dtsi
  ARM: dts: Change i2s compatible string on exynos5250

 .../devicetree/bindings/sound/samsung-i2s.txt      |   22 ++--
 arch/arm/boot/dts/exynos5.dtsi                     |   21 +++
 arch/arm/boot/dts/exynos5250.dtsi                  |   17 +---
 include/linux/platform_data/asoc-s3c.h             |    1 +
 sound/soc/samsung/i2s-regs.h                       |   15 ++
 sound/soc/samsung/i2s.c                            |  143 +++++++++++++++-----
 6 files changed, 157 insertions(+), 62 deletions(-)

-- 
1.7.4.4

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

* [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
  2013-08-12  9:49 ` Padmavathi Venna
@ 2013-08-12  9:49   ` Padmavathi Venna
  -1 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, alsa-devel, devicetree,
	padma.v, padma.kvr
  Cc: broonie, kgene.kim, tomasz.figa, abrestic

Samsung has different versions of I2S introduced in different
platforms. Each version has some new support added for multichannel,
secondary fifo, s/w reset control and internal mux for rclk src clk.
Each newly added change has a quirk. So this patch adds all the
required quirks as driver data and based on compatible string from
dtsi fetches the quirks.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 .../devicetree/bindings/sound/samsung-i2s.txt      |   18 ++----
 sound/soc/samsung/i2s.c                            |   62 +++++++++++--------
 2 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
index 025e66b..25a0024 100644
--- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
@@ -2,7 +2,11 @@
 
 Required SoC Specific Properties:
 
-- compatible : "samsung,i2s-v5"
+- compatible : should be one of the following.
+   - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
+   - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
+     secondary fifo, s/w reset control and internal mux for root clk src.
+
 - reg: physical base address of the controller and length of memory mapped
   region.
 - dmas: list of DMA controller phandle and DMA request line ordered pairs.
@@ -21,13 +25,6 @@ Required SoC Specific Properties:
 
 Optional SoC Specific Properties:
 
-- samsung,supports-6ch: If the I2S Primary sound source has 5.1 Channel
-  support, this flag is enabled.
-- samsung,supports-rstclr: This flag should be set if I2S software reset bit
-  control is required. When this flag is set I2S software reset bit will be
-  enabled or disabled based on need.
-- samsung,supports-secdai:If I2S block has a secondary FIFO and internal DMA,
-  then this flag is enabled.
 - 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.
@@ -36,7 +33,7 @@ Optional SoC Specific Properties:
 Example:
 
 i2s0: i2s@03830000 {
-	compatible = "samsung,i2s-v5";
+	compatible = "samsung,s5pv210-i2s";
 	reg = <0x03830000 0x100>;
 	dmas = <&pdma0 10
 		&pdma0 9
@@ -46,9 +43,6 @@ i2s0: i2s@03830000 {
 		<&clock_audss EXYNOS_I2S_BUS>,
 		<&clock_audss EXYNOS_SCLK_I2S>;
 	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
-	samsung,supports-6ch;
-	samsung,supports-rstclr;
-	samsung,supports-secdai;
 	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 47e08dd..1671d9b 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -40,6 +40,7 @@ enum samsung_dai_type {
 
 struct samsung_i2s_dai_data {
 	int dai_type;
+	u32 quirks;
 };
 
 struct i2s_dai {
@@ -1032,18 +1033,18 @@ static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev, bool sec)
 
 static const struct of_device_id exynos_i2s_match[];
 
-static inline int samsung_i2s_get_driver_data(struct platform_device *pdev)
+static inline const struct samsung_i2s_dai_data *samsung_i2s_get_driver_data(
+						struct platform_device *pdev)
 {
 #ifdef CONFIG_OF
-	struct samsung_i2s_dai_data *data;
 	if (pdev->dev.of_node) {
 		const struct of_device_id *match;
 		match = of_match_node(exynos_i2s_match, pdev->dev.of_node);
-		data = (struct samsung_i2s_dai_data *) match->data;
-		return data->dai_type;
+		return match->data;
 	} else
 #endif
-		return platform_get_device_id(pdev)->driver_data;
+		return (struct samsung_i2s_dai_data *)
+				platform_get_device_id(pdev)->driver_data;
 }
 
 #ifdef CONFIG_PM_RUNTIME
@@ -1074,13 +1075,13 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 	struct resource *res;
 	u32 regs_base, quirks = 0, idma_addr = 0;
 	struct device_node *np = pdev->dev.of_node;
-	enum samsung_dai_type samsung_dai_type;
+	const struct samsung_i2s_dai_data *i2s_dai_data;
 	int ret = 0;
 
 	/* Call during Seconday interface registration */
-	samsung_dai_type = samsung_i2s_get_driver_data(pdev);
+	i2s_dai_data = samsung_i2s_get_driver_data(pdev);
 
-	if (samsung_dai_type == TYPE_SEC) {
+	if (i2s_dai_data->dai_type == TYPE_SEC) {
 		sec_dai = dev_get_drvdata(&pdev->dev);
 		if (!sec_dai) {
 			dev_err(&pdev->dev, "Unable to get drvdata\n");
@@ -1129,15 +1130,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 			idma_addr = i2s_cfg->idma_addr;
 		}
 	} else {
-		if (of_find_property(np, "samsung,supports-6ch", NULL))
-			quirks |= QUIRK_PRI_6CHAN;
-
-		if (of_find_property(np, "samsung,supports-secdai", NULL))
-			quirks |= QUIRK_SEC_DAI;
-
-		if (of_find_property(np, "samsung,supports-rstclr", NULL))
-			quirks |= QUIRK_NEED_RSTCLR;
-
+		quirks = i2s_dai_data->quirks;
 		if (of_property_read_u32(np, "samsung,idma-addr",
 					 &idma_addr)) {
 			if (quirks & QUIRK_SEC_DAI) {
@@ -1250,27 +1243,44 @@ static int samsung_i2s_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct samsung_i2s_dai_data i2sv3_dai_type = {
+	.dai_type = TYPE_PRI,
+	.quirks = QUIRK_NO_MUXPSR,
+};
+
+static const struct samsung_i2s_dai_data i2sv5_dai_type = {
+	.dai_type = TYPE_PRI,
+	.quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR,
+};
+
+static const struct samsung_i2s_dai_data samsung_dai_type_pri = {
+	.dai_type = TYPE_PRI,
+};
+
+static const struct samsung_i2s_dai_data samsung_dai_type_sec = {
+	.dai_type = TYPE_SEC,
+};
+
 static struct platform_device_id samsung_i2s_driver_ids[] = {
 	{
 		.name           = "samsung-i2s",
-		.driver_data	= TYPE_PRI,
+		.driver_data    = (kernel_ulong_t)&samsung_dai_type_pri,
 	}, {
 		.name           = "samsung-i2s-sec",
-		.driver_data	= TYPE_SEC,
+		.driver_data    = (kernel_ulong_t)&samsung_dai_type_sec,
 	},
 	{},
 };
 MODULE_DEVICE_TABLE(platform, samsung_i2s_driver_ids);
 
 #ifdef CONFIG_OF
-static struct samsung_i2s_dai_data samsung_i2s_dai_data_array[] = {
-	[TYPE_PRI] = { TYPE_PRI },
-	[TYPE_SEC] = { TYPE_SEC },
-};
-
 static const struct of_device_id exynos_i2s_match[] = {
-	{ .compatible = "samsung,i2s-v5",
-	  .data = &samsung_i2s_dai_data_array[TYPE_PRI],
+	{
+		.compatible = "samsung,s3c6410-i2s",
+		.data = &i2sv3_dai_type,
+	}, {
+		.compatible = "samsung,s5pv210-i2s",
+		.data = &i2sv5_dai_type,
 	},
 	{},
 };
-- 
1.7.4.4

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

* [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
@ 2013-08-12  9:49   ` Padmavathi Venna
  0 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-arm-kernel

Samsung has different versions of I2S introduced in different
platforms. Each version has some new support added for multichannel,
secondary fifo, s/w reset control and internal mux for rclk src clk.
Each newly added change has a quirk. So this patch adds all the
required quirks as driver data and based on compatible string from
dtsi fetches the quirks.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
---
 .../devicetree/bindings/sound/samsung-i2s.txt      |   18 ++----
 sound/soc/samsung/i2s.c                            |   62 +++++++++++--------
 2 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
index 025e66b..25a0024 100644
--- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
@@ -2,7 +2,11 @@
 
 Required SoC Specific Properties:
 
-- compatible : "samsung,i2s-v5"
+- compatible : should be one of the following.
+   - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
+   - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
+     secondary fifo, s/w reset control and internal mux for root clk src.
+
 - reg: physical base address of the controller and length of memory mapped
   region.
 - dmas: list of DMA controller phandle and DMA request line ordered pairs.
@@ -21,13 +25,6 @@ Required SoC Specific Properties:
 
 Optional SoC Specific Properties:
 
-- samsung,supports-6ch: If the I2S Primary sound source has 5.1 Channel
-  support, this flag is enabled.
-- samsung,supports-rstclr: This flag should be set if I2S software reset bit
-  control is required. When this flag is set I2S software reset bit will be
-  enabled or disabled based on need.
-- samsung,supports-secdai:If I2S block has a secondary FIFO and internal DMA,
-  then this flag is enabled.
 - 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.
@@ -36,7 +33,7 @@ Optional SoC Specific Properties:
 Example:
 
 i2s0: i2s at 03830000 {
-	compatible = "samsung,i2s-v5";
+	compatible = "samsung,s5pv210-i2s";
 	reg = <0x03830000 0x100>;
 	dmas = <&pdma0 10
 		&pdma0 9
@@ -46,9 +43,6 @@ i2s0: i2s@03830000 {
 		<&clock_audss EXYNOS_I2S_BUS>,
 		<&clock_audss EXYNOS_SCLK_I2S>;
 	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
-	samsung,supports-6ch;
-	samsung,supports-rstclr;
-	samsung,supports-secdai;
 	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 47e08dd..1671d9b 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -40,6 +40,7 @@ enum samsung_dai_type {
 
 struct samsung_i2s_dai_data {
 	int dai_type;
+	u32 quirks;
 };
 
 struct i2s_dai {
@@ -1032,18 +1033,18 @@ static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev, bool sec)
 
 static const struct of_device_id exynos_i2s_match[];
 
-static inline int samsung_i2s_get_driver_data(struct platform_device *pdev)
+static inline const struct samsung_i2s_dai_data *samsung_i2s_get_driver_data(
+						struct platform_device *pdev)
 {
 #ifdef CONFIG_OF
-	struct samsung_i2s_dai_data *data;
 	if (pdev->dev.of_node) {
 		const struct of_device_id *match;
 		match = of_match_node(exynos_i2s_match, pdev->dev.of_node);
-		data = (struct samsung_i2s_dai_data *) match->data;
-		return data->dai_type;
+		return match->data;
 	} else
 #endif
-		return platform_get_device_id(pdev)->driver_data;
+		return (struct samsung_i2s_dai_data *)
+				platform_get_device_id(pdev)->driver_data;
 }
 
 #ifdef CONFIG_PM_RUNTIME
@@ -1074,13 +1075,13 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 	struct resource *res;
 	u32 regs_base, quirks = 0, idma_addr = 0;
 	struct device_node *np = pdev->dev.of_node;
-	enum samsung_dai_type samsung_dai_type;
+	const struct samsung_i2s_dai_data *i2s_dai_data;
 	int ret = 0;
 
 	/* Call during Seconday interface registration */
-	samsung_dai_type = samsung_i2s_get_driver_data(pdev);
+	i2s_dai_data = samsung_i2s_get_driver_data(pdev);
 
-	if (samsung_dai_type == TYPE_SEC) {
+	if (i2s_dai_data->dai_type == TYPE_SEC) {
 		sec_dai = dev_get_drvdata(&pdev->dev);
 		if (!sec_dai) {
 			dev_err(&pdev->dev, "Unable to get drvdata\n");
@@ -1129,15 +1130,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 			idma_addr = i2s_cfg->idma_addr;
 		}
 	} else {
-		if (of_find_property(np, "samsung,supports-6ch", NULL))
-			quirks |= QUIRK_PRI_6CHAN;
-
-		if (of_find_property(np, "samsung,supports-secdai", NULL))
-			quirks |= QUIRK_SEC_DAI;
-
-		if (of_find_property(np, "samsung,supports-rstclr", NULL))
-			quirks |= QUIRK_NEED_RSTCLR;
-
+		quirks = i2s_dai_data->quirks;
 		if (of_property_read_u32(np, "samsung,idma-addr",
 					 &idma_addr)) {
 			if (quirks & QUIRK_SEC_DAI) {
@@ -1250,27 +1243,44 @@ static int samsung_i2s_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct samsung_i2s_dai_data i2sv3_dai_type = {
+	.dai_type = TYPE_PRI,
+	.quirks = QUIRK_NO_MUXPSR,
+};
+
+static const struct samsung_i2s_dai_data i2sv5_dai_type = {
+	.dai_type = TYPE_PRI,
+	.quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR,
+};
+
+static const struct samsung_i2s_dai_data samsung_dai_type_pri = {
+	.dai_type = TYPE_PRI,
+};
+
+static const struct samsung_i2s_dai_data samsung_dai_type_sec = {
+	.dai_type = TYPE_SEC,
+};
+
 static struct platform_device_id samsung_i2s_driver_ids[] = {
 	{
 		.name           = "samsung-i2s",
-		.driver_data	= TYPE_PRI,
+		.driver_data    = (kernel_ulong_t)&samsung_dai_type_pri,
 	}, {
 		.name           = "samsung-i2s-sec",
-		.driver_data	= TYPE_SEC,
+		.driver_data    = (kernel_ulong_t)&samsung_dai_type_sec,
 	},
 	{},
 };
 MODULE_DEVICE_TABLE(platform, samsung_i2s_driver_ids);
 
 #ifdef CONFIG_OF
-static struct samsung_i2s_dai_data samsung_i2s_dai_data_array[] = {
-	[TYPE_PRI] = { TYPE_PRI },
-	[TYPE_SEC] = { TYPE_SEC },
-};
-
 static const struct of_device_id exynos_i2s_match[] = {
-	{ .compatible = "samsung,i2s-v5",
-	  .data = &samsung_i2s_dai_data_array[TYPE_PRI],
+	{
+		.compatible = "samsung,s3c6410-i2s",
+		.data = &i2sv3_dai_type,
+	}, {
+		.compatible = "samsung,s5pv210-i2s",
+		.data = &i2sv5_dai_type,
 	},
 	{},
 };
-- 
1.7.4.4

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

* [PATCH V4 2/4] ASoC: Samsung: I2S: Modify the I2S driver to support I2S on Exynos5420
  2013-08-12  9:49 ` Padmavathi Venna
@ 2013-08-12  9:49   ` Padmavathi Venna
  -1 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, alsa-devel, devicetree,
	padma.v, padma.kvr
  Cc: broonie, kgene.kim, tomasz.figa, abrestic

Exynos5420 added support for I2S TDM mode. For this, there are some
register changes in the I2S controller. This patch adds the relevant
register changes to support I2S in normal mode. This patch adds a
quirk for TDM mode and if TDM mode is present all the relevent changes
will be applied.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
 .../devicetree/bindings/sound/samsung-i2s.txt      |    4 +
 include/linux/platform_data/asoc-s3c.h             |    1 +
 sound/soc/samsung/i2s-regs.h                       |   15 ++++
 sound/soc/samsung/i2s.c                            |   81 ++++++++++++++++++--
 4 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
index 25a0024..7386d44 100644
--- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
@@ -6,6 +6,10 @@ Required SoC Specific Properties:
    - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
    - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
      secondary fifo, s/w reset control and internal mux for root clk src.
+   - samsung,exynos5420-i2s: for 8/16/24bit multichannel(7.1) I2S with
+     secondary fifo, s/w reset control, internal mux for root clk src and
+     TDM support. TDM (Time division multiplexing) is to allow transfer of
+     multiple channel audio data on single data line.
 
 - reg: physical base address of the controller and length of memory mapped
   region.
diff --git a/include/linux/platform_data/asoc-s3c.h b/include/linux/platform_data/asoc-s3c.h
index 8827259..9efc04d 100644
--- a/include/linux/platform_data/asoc-s3c.h
+++ b/include/linux/platform_data/asoc-s3c.h
@@ -36,6 +36,7 @@ struct samsung_i2s {
  */
 #define QUIRK_NO_MUXPSR		(1 << 2)
 #define QUIRK_NEED_RSTCLR	(1 << 3)
+#define QUIRK_SUPPORTS_TDM	(1 << 4)
 	/* Quirks of the I2S controller */
 	u32 quirks;
 	dma_addr_t idma_addr;
diff --git a/sound/soc/samsung/i2s-regs.h b/sound/soc/samsung/i2s-regs.h
index 30513b7..821a502 100644
--- a/sound/soc/samsung/i2s-regs.h
+++ b/sound/soc/samsung/i2s-regs.h
@@ -31,6 +31,10 @@
 #define I2SLVL1ADDR	0x34
 #define I2SLVL2ADDR	0x38
 #define I2SLVL3ADDR	0x3c
+#define I2SSTR1		0x40
+#define I2SVER		0x44
+#define I2SFIC2		0x48
+#define I2STDM		0x4c
 
 #define CON_RSTCLR		(1 << 31)
 #define CON_FRXOFSTATUS		(1 << 26)
@@ -117,6 +121,17 @@
 #define MOD_BCLK_MASK		3
 #define MOD_8BIT		(1 << 0)
 
+#define EXYNOS5420_MOD_LRP_SHIFT	15
+#define EXYNOS5420_MOD_SDF_SHIFT	6
+#define EXYNOS5420_MOD_RCLK_SHIFT	4
+#define EXYNOS5420_MOD_BCLK_SHIFT	0
+#define EXYNOS5420_MOD_BCLK_64FS	4
+#define EXYNOS5420_MOD_BCLK_96FS	5
+#define EXYNOS5420_MOD_BCLK_128FS	6
+#define EXYNOS5420_MOD_BCLK_192FS	7
+#define EXYNOS5420_MOD_BCLK_256FS	8
+#define EXYNOS5420_MOD_BCLK_MASK	0xf
+
 #define MOD_CDCLKCON		(1 << 12)
 
 #define PSR_PSREN		(1 << 15)
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 1671d9b..ce487c2 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -199,7 +199,12 @@ static inline bool is_manager(struct i2s_dai *i2s)
 /* Read RCLK of I2S (in multiples of LRCLK) */
 static inline unsigned get_rfs(struct i2s_dai *i2s)
 {
-	u32 rfs = (readl(i2s->addr + I2SMOD) >> MOD_RCLK_SHIFT);
+	u32 rfs;
+
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM)
+		rfs = readl(i2s->addr + I2SMOD) >> EXYNOS5420_MOD_RCLK_SHIFT;
+	else
+		rfs = (readl(i2s->addr + I2SMOD) >> MOD_RCLK_SHIFT);
 	rfs &= MOD_RCLK_MASK;
 
 	switch (rfs) {
@@ -214,8 +219,12 @@ static inline unsigned get_rfs(struct i2s_dai *i2s)
 static inline void set_rfs(struct i2s_dai *i2s, unsigned rfs)
 {
 	u32 mod = readl(i2s->addr + I2SMOD);
-	int rfs_shift =  MOD_RCLK_SHIFT;
+	int rfs_shift;
 
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM)
+		rfs_shift = EXYNOS5420_MOD_RCLK_SHIFT;
+	else
+		rfs_shift = MOD_RCLK_SHIFT;
 	mod &= ~(MOD_RCLK_MASK << rfs_shift);
 
 	switch (rfs) {
@@ -239,10 +248,22 @@ static inline void set_rfs(struct i2s_dai *i2s, unsigned rfs)
 /* Read Bit-Clock of I2S (in multiples of LRCLK) */
 static inline unsigned get_bfs(struct i2s_dai *i2s)
 {
-	u32 bfs =  readl(i2s->addr + I2SMOD) >> MOD_BCLK_SHIFT;
-	bfs &= MOD_BCLK_MASK;
+	u32 bfs;
+
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM) {
+		bfs = readl(i2s->addr + I2SMOD) >> EXYNOS5420_MOD_BCLK_SHIFT;
+		bfs &= EXYNOS5420_MOD_BCLK_MASK;
+	} else {
+		bfs =  readl(i2s->addr + I2SMOD) >> MOD_BCLK_SHIFT;
+		bfs &= MOD_BCLK_MASK;
+	}
 
 	switch (bfs) {
+	case 8: return 256;
+	case 7: return 192;
+	case 6: return 128;
+	case 5: return 96;
+	case 4: return 64;
 	case 3: return 24;
 	case 2: return 16;
 	case 1:	return 48;
@@ -254,9 +275,22 @@ static inline unsigned get_bfs(struct i2s_dai *i2s)
 static inline void set_bfs(struct i2s_dai *i2s, unsigned bfs)
 {
 	u32 mod = readl(i2s->addr + I2SMOD);
-	int bfs_shift = MOD_BCLK_SHIFT;
+	int bfs_shift;
+	int tdm = i2s->quirks & QUIRK_SUPPORTS_TDM;
 
-	mod &= ~(MOD_BCLK_MASK << bfs_shift);
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM) {
+		bfs_shift = EXYNOS5420_MOD_BCLK_SHIFT;
+		mod &= ~(EXYNOS5420_MOD_BCLK_MASK << bfs_shift);
+	} else {
+		bfs_shift = MOD_BCLK_SHIFT;
+		mod &= ~(MOD_BCLK_MASK << bfs_shift);
+	}
+
+	/* Non-TDM I2S controllers do not support BCLK > 48 * FS */
+	if (!tdm && bfs > 48) {
+		dev_err(&i2s->pdev->dev, "Unsupported BCLK divider\n");
+		return;
+	}
 
 	switch (bfs) {
 	case 48:
@@ -271,6 +305,21 @@ static inline void set_bfs(struct i2s_dai *i2s, unsigned bfs)
 	case 16:
 		mod |= (MOD_BCLK_16FS << bfs_shift);
 		break;
+	case 64:
+		mod |= (EXYNOS5420_MOD_BCLK_64FS << bfs_shift);
+		break;
+	case 96:
+		mod |= (EXYNOS5420_MOD_BCLK_96FS << bfs_shift);
+		break;
+	case 128:
+		mod |= (EXYNOS5420_MOD_BCLK_128FS << bfs_shift);
+		break;
+	case 192:
+		mod |= (EXYNOS5420_MOD_BCLK_192FS << bfs_shift);
+		break;
+	case 256:
+		mod |= (EXYNOS5420_MOD_BCLK_256FS << bfs_shift);
+		break;
 	default:
 		dev_err(&i2s->pdev->dev, "Wrong BCLK Divider!\n");
 		return;
@@ -496,10 +545,17 @@ static int i2s_set_fmt(struct snd_soc_dai *dai,
 {
 	struct i2s_dai *i2s = to_info(dai);
 	u32 mod = readl(i2s->addr + I2SMOD);
-	int lrp_shift = MOD_LRP_SHIFT, sdf_shift = MOD_SDF_SHIFT;
-	int sdf_mask, lrp_rlow;
+	int lrp_shift, sdf_shift, sdf_mask, lrp_rlow;
 	u32 tmp = 0;
 
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM) {
+		lrp_shift = EXYNOS5420_MOD_LRP_SHIFT;
+		sdf_shift = EXYNOS5420_MOD_SDF_SHIFT;
+	} else {
+		lrp_shift = MOD_LRP_SHIFT;
+		sdf_shift = MOD_SDF_SHIFT;
+	}
+
 	sdf_mask = MOD_SDF_MASK << sdf_shift;
 	lrp_rlow = MOD_LR_RLOW << lrp_shift;
 
@@ -1253,6 +1309,12 @@ static const struct samsung_i2s_dai_data i2sv5_dai_type = {
 	.quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR,
 };
 
+static const struct samsung_i2s_dai_data i2sv6_dai_type = {
+	.dai_type = TYPE_PRI,
+	.quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR |
+			QUIRK_SUPPORTS_TDM,
+};
+
 static const struct samsung_i2s_dai_data samsung_dai_type_pri = {
 	.dai_type = TYPE_PRI,
 };
@@ -1281,6 +1343,9 @@ static const struct of_device_id exynos_i2s_match[] = {
 	}, {
 		.compatible = "samsung,s5pv210-i2s",
 		.data = &i2sv5_dai_type,
+	}, {
+		.compatible = "samsung,exynos5420-i2s",
+		.data = &i2sv6_dai_type,
 	},
 	{},
 };
-- 
1.7.4.4

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

* [PATCH V4 2/4] ASoC: Samsung: I2S: Modify the I2S driver to support I2S on Exynos5420
@ 2013-08-12  9:49   ` Padmavathi Venna
  0 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-arm-kernel

Exynos5420 added support for I2S TDM mode. For this, there are some
register changes in the I2S controller. This patch adds the relevant
register changes to support I2S in normal mode. This patch adds a
quirk for TDM mode and if TDM mode is present all the relevent changes
will be applied.

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
---
 .../devicetree/bindings/sound/samsung-i2s.txt      |    4 +
 include/linux/platform_data/asoc-s3c.h             |    1 +
 sound/soc/samsung/i2s-regs.h                       |   15 ++++
 sound/soc/samsung/i2s.c                            |   81 ++++++++++++++++++--
 4 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
index 25a0024..7386d44 100644
--- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
@@ -6,6 +6,10 @@ Required SoC Specific Properties:
    - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
    - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
      secondary fifo, s/w reset control and internal mux for root clk src.
+   - samsung,exynos5420-i2s: for 8/16/24bit multichannel(7.1) I2S with
+     secondary fifo, s/w reset control, internal mux for root clk src and
+     TDM support. TDM (Time division multiplexing) is to allow transfer of
+     multiple channel audio data on single data line.
 
 - reg: physical base address of the controller and length of memory mapped
   region.
diff --git a/include/linux/platform_data/asoc-s3c.h b/include/linux/platform_data/asoc-s3c.h
index 8827259..9efc04d 100644
--- a/include/linux/platform_data/asoc-s3c.h
+++ b/include/linux/platform_data/asoc-s3c.h
@@ -36,6 +36,7 @@ struct samsung_i2s {
  */
 #define QUIRK_NO_MUXPSR		(1 << 2)
 #define QUIRK_NEED_RSTCLR	(1 << 3)
+#define QUIRK_SUPPORTS_TDM	(1 << 4)
 	/* Quirks of the I2S controller */
 	u32 quirks;
 	dma_addr_t idma_addr;
diff --git a/sound/soc/samsung/i2s-regs.h b/sound/soc/samsung/i2s-regs.h
index 30513b7..821a502 100644
--- a/sound/soc/samsung/i2s-regs.h
+++ b/sound/soc/samsung/i2s-regs.h
@@ -31,6 +31,10 @@
 #define I2SLVL1ADDR	0x34
 #define I2SLVL2ADDR	0x38
 #define I2SLVL3ADDR	0x3c
+#define I2SSTR1		0x40
+#define I2SVER		0x44
+#define I2SFIC2		0x48
+#define I2STDM		0x4c
 
 #define CON_RSTCLR		(1 << 31)
 #define CON_FRXOFSTATUS		(1 << 26)
@@ -117,6 +121,17 @@
 #define MOD_BCLK_MASK		3
 #define MOD_8BIT		(1 << 0)
 
+#define EXYNOS5420_MOD_LRP_SHIFT	15
+#define EXYNOS5420_MOD_SDF_SHIFT	6
+#define EXYNOS5420_MOD_RCLK_SHIFT	4
+#define EXYNOS5420_MOD_BCLK_SHIFT	0
+#define EXYNOS5420_MOD_BCLK_64FS	4
+#define EXYNOS5420_MOD_BCLK_96FS	5
+#define EXYNOS5420_MOD_BCLK_128FS	6
+#define EXYNOS5420_MOD_BCLK_192FS	7
+#define EXYNOS5420_MOD_BCLK_256FS	8
+#define EXYNOS5420_MOD_BCLK_MASK	0xf
+
 #define MOD_CDCLKCON		(1 << 12)
 
 #define PSR_PSREN		(1 << 15)
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 1671d9b..ce487c2 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -199,7 +199,12 @@ static inline bool is_manager(struct i2s_dai *i2s)
 /* Read RCLK of I2S (in multiples of LRCLK) */
 static inline unsigned get_rfs(struct i2s_dai *i2s)
 {
-	u32 rfs = (readl(i2s->addr + I2SMOD) >> MOD_RCLK_SHIFT);
+	u32 rfs;
+
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM)
+		rfs = readl(i2s->addr + I2SMOD) >> EXYNOS5420_MOD_RCLK_SHIFT;
+	else
+		rfs = (readl(i2s->addr + I2SMOD) >> MOD_RCLK_SHIFT);
 	rfs &= MOD_RCLK_MASK;
 
 	switch (rfs) {
@@ -214,8 +219,12 @@ static inline unsigned get_rfs(struct i2s_dai *i2s)
 static inline void set_rfs(struct i2s_dai *i2s, unsigned rfs)
 {
 	u32 mod = readl(i2s->addr + I2SMOD);
-	int rfs_shift =  MOD_RCLK_SHIFT;
+	int rfs_shift;
 
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM)
+		rfs_shift = EXYNOS5420_MOD_RCLK_SHIFT;
+	else
+		rfs_shift = MOD_RCLK_SHIFT;
 	mod &= ~(MOD_RCLK_MASK << rfs_shift);
 
 	switch (rfs) {
@@ -239,10 +248,22 @@ static inline void set_rfs(struct i2s_dai *i2s, unsigned rfs)
 /* Read Bit-Clock of I2S (in multiples of LRCLK) */
 static inline unsigned get_bfs(struct i2s_dai *i2s)
 {
-	u32 bfs =  readl(i2s->addr + I2SMOD) >> MOD_BCLK_SHIFT;
-	bfs &= MOD_BCLK_MASK;
+	u32 bfs;
+
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM) {
+		bfs = readl(i2s->addr + I2SMOD) >> EXYNOS5420_MOD_BCLK_SHIFT;
+		bfs &= EXYNOS5420_MOD_BCLK_MASK;
+	} else {
+		bfs =  readl(i2s->addr + I2SMOD) >> MOD_BCLK_SHIFT;
+		bfs &= MOD_BCLK_MASK;
+	}
 
 	switch (bfs) {
+	case 8: return 256;
+	case 7: return 192;
+	case 6: return 128;
+	case 5: return 96;
+	case 4: return 64;
 	case 3: return 24;
 	case 2: return 16;
 	case 1:	return 48;
@@ -254,9 +275,22 @@ static inline unsigned get_bfs(struct i2s_dai *i2s)
 static inline void set_bfs(struct i2s_dai *i2s, unsigned bfs)
 {
 	u32 mod = readl(i2s->addr + I2SMOD);
-	int bfs_shift = MOD_BCLK_SHIFT;
+	int bfs_shift;
+	int tdm = i2s->quirks & QUIRK_SUPPORTS_TDM;
 
-	mod &= ~(MOD_BCLK_MASK << bfs_shift);
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM) {
+		bfs_shift = EXYNOS5420_MOD_BCLK_SHIFT;
+		mod &= ~(EXYNOS5420_MOD_BCLK_MASK << bfs_shift);
+	} else {
+		bfs_shift = MOD_BCLK_SHIFT;
+		mod &= ~(MOD_BCLK_MASK << bfs_shift);
+	}
+
+	/* Non-TDM I2S controllers do not support BCLK > 48 * FS */
+	if (!tdm && bfs > 48) {
+		dev_err(&i2s->pdev->dev, "Unsupported BCLK divider\n");
+		return;
+	}
 
 	switch (bfs) {
 	case 48:
@@ -271,6 +305,21 @@ static inline void set_bfs(struct i2s_dai *i2s, unsigned bfs)
 	case 16:
 		mod |= (MOD_BCLK_16FS << bfs_shift);
 		break;
+	case 64:
+		mod |= (EXYNOS5420_MOD_BCLK_64FS << bfs_shift);
+		break;
+	case 96:
+		mod |= (EXYNOS5420_MOD_BCLK_96FS << bfs_shift);
+		break;
+	case 128:
+		mod |= (EXYNOS5420_MOD_BCLK_128FS << bfs_shift);
+		break;
+	case 192:
+		mod |= (EXYNOS5420_MOD_BCLK_192FS << bfs_shift);
+		break;
+	case 256:
+		mod |= (EXYNOS5420_MOD_BCLK_256FS << bfs_shift);
+		break;
 	default:
 		dev_err(&i2s->pdev->dev, "Wrong BCLK Divider!\n");
 		return;
@@ -496,10 +545,17 @@ static int i2s_set_fmt(struct snd_soc_dai *dai,
 {
 	struct i2s_dai *i2s = to_info(dai);
 	u32 mod = readl(i2s->addr + I2SMOD);
-	int lrp_shift = MOD_LRP_SHIFT, sdf_shift = MOD_SDF_SHIFT;
-	int sdf_mask, lrp_rlow;
+	int lrp_shift, sdf_shift, sdf_mask, lrp_rlow;
 	u32 tmp = 0;
 
+	if (i2s->quirks & QUIRK_SUPPORTS_TDM) {
+		lrp_shift = EXYNOS5420_MOD_LRP_SHIFT;
+		sdf_shift = EXYNOS5420_MOD_SDF_SHIFT;
+	} else {
+		lrp_shift = MOD_LRP_SHIFT;
+		sdf_shift = MOD_SDF_SHIFT;
+	}
+
 	sdf_mask = MOD_SDF_MASK << sdf_shift;
 	lrp_rlow = MOD_LR_RLOW << lrp_shift;
 
@@ -1253,6 +1309,12 @@ static const struct samsung_i2s_dai_data i2sv5_dai_type = {
 	.quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR,
 };
 
+static const struct samsung_i2s_dai_data i2sv6_dai_type = {
+	.dai_type = TYPE_PRI,
+	.quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR |
+			QUIRK_SUPPORTS_TDM,
+};
+
 static const struct samsung_i2s_dai_data samsung_dai_type_pri = {
 	.dai_type = TYPE_PRI,
 };
@@ -1281,6 +1343,9 @@ static const struct of_device_id exynos_i2s_match[] = {
 	}, {
 		.compatible = "samsung,s5pv210-i2s",
 		.data = &i2sv5_dai_type,
+	}, {
+		.compatible = "samsung,exynos5420-i2s",
+		.data = &i2sv6_dai_type,
 	},
 	{},
 };
-- 
1.7.4.4

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

* [PATCH V4 3/4] ARM: dts: exynos5250: move common i2s properties to exynos5 dtsi
  2013-08-12  9:49 ` Padmavathi Venna
@ 2013-08-12  9:49   ` Padmavathi Venna
  -1 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, alsa-devel, devicetree,
	padma.v, padma.kvr
  Cc: broonie, kgene.kim, tomasz.figa, abrestic

I2S nodes shares some properties across exynos5 SoCs (exynos5250
and exyno5420). Common code is moved to exynos5.dtsi which is
included in exyno5250 and exynos5420 SoC files.

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

diff --git a/arch/arm/boot/dts/exynos5.dtsi b/arch/arm/boot/dts/exynos5.dtsi
index f65e124..aae2fa1 100644
--- a/arch/arm/boot/dts/exynos5.dtsi
+++ b/arch/arm/boot/dts/exynos5.dtsi
@@ -108,4 +108,25 @@
 		interrupts = <0 42 0>;
 		status = "disabled";
 	};
+
+	i2s0: i2s@03830000 {
+		reg = <0x03830000 0x100>;
+		samsung,idma-addr = <0x03000000>;
+	};
+
+	i2s1: i2s@12D60000 {
+		compatible = "samsung,i2s-v5";
+		reg = <0x12D60000 0x100>;
+		dmas = <&pdma1 12
+			&pdma1 11>;
+		dma-names = "tx", "rx";
+	};
+
+	i2s2: i2s@12D70000 {
+		compatible = "samsung,i2s-v5";
+		reg = <0x12D70000 0x100>;
+		dmas = <&pdma0 12
+			&pdma0 11>;
+		dma-names = "tx", "rx";
+	};
 };
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index ef57277..f941d52 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -406,7 +406,6 @@
 
 	i2s0: i2s@03830000 {
 		compatible = "samsung,i2s-v5";
-		reg = <0x03830000 0x100>;
 		dmas = <&pdma0 10
 			&pdma0 9
 			&pdma0 8>;
@@ -418,17 +417,11 @@
 		samsung,supports-6ch;
 		samsung,supports-rstclr;
 		samsung,supports-secdai;
-		samsung,idma-addr = <0x03000000>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s0_bus>;
 	};
 
 	i2s1: i2s@12D60000 {
-		compatible = "samsung,i2s-v5";
-		reg = <0x12D60000 0x100>;
-		dmas = <&pdma1 12
-			&pdma1 11>;
-		dma-names = "tx", "rx";
 		clocks = <&clock 307>, <&clock 157>;
 		clock-names = "iis", "i2s_opclk0";
 		pinctrl-names = "default";
@@ -436,11 +429,6 @@
 	};
 
 	i2s2: i2s@12D70000 {
-		compatible = "samsung,i2s-v5";
-		reg = <0x12D70000 0x100>;
-		dmas = <&pdma0 12
-			&pdma0 11>;
-		dma-names = "tx", "rx";
 		clocks = <&clock 308>, <&clock 158>;
 		clock-names = "iis", "i2s_opclk0";
 		pinctrl-names = "default";
-- 
1.7.4.4

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

* [PATCH V4 3/4] ARM: dts: exynos5250: move common i2s properties to exynos5 dtsi
@ 2013-08-12  9:49   ` Padmavathi Venna
  0 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-arm-kernel

I2S nodes shares some properties across exynos5 SoCs (exynos5250
and exyno5420). Common code is moved to exynos5.dtsi which is
included in exyno5250 and exynos5420 SoC files.

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

diff --git a/arch/arm/boot/dts/exynos5.dtsi b/arch/arm/boot/dts/exynos5.dtsi
index f65e124..aae2fa1 100644
--- a/arch/arm/boot/dts/exynos5.dtsi
+++ b/arch/arm/boot/dts/exynos5.dtsi
@@ -108,4 +108,25 @@
 		interrupts = <0 42 0>;
 		status = "disabled";
 	};
+
+	i2s0: i2s at 03830000 {
+		reg = <0x03830000 0x100>;
+		samsung,idma-addr = <0x03000000>;
+	};
+
+	i2s1: i2s at 12D60000 {
+		compatible = "samsung,i2s-v5";
+		reg = <0x12D60000 0x100>;
+		dmas = <&pdma1 12
+			&pdma1 11>;
+		dma-names = "tx", "rx";
+	};
+
+	i2s2: i2s at 12D70000 {
+		compatible = "samsung,i2s-v5";
+		reg = <0x12D70000 0x100>;
+		dmas = <&pdma0 12
+			&pdma0 11>;
+		dma-names = "tx", "rx";
+	};
 };
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index ef57277..f941d52 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -406,7 +406,6 @@
 
 	i2s0: i2s at 03830000 {
 		compatible = "samsung,i2s-v5";
-		reg = <0x03830000 0x100>;
 		dmas = <&pdma0 10
 			&pdma0 9
 			&pdma0 8>;
@@ -418,17 +417,11 @@
 		samsung,supports-6ch;
 		samsung,supports-rstclr;
 		samsung,supports-secdai;
-		samsung,idma-addr = <0x03000000>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s0_bus>;
 	};
 
 	i2s1: i2s at 12D60000 {
-		compatible = "samsung,i2s-v5";
-		reg = <0x12D60000 0x100>;
-		dmas = <&pdma1 12
-			&pdma1 11>;
-		dma-names = "tx", "rx";
 		clocks = <&clock 307>, <&clock 157>;
 		clock-names = "iis", "i2s_opclk0";
 		pinctrl-names = "default";
@@ -436,11 +429,6 @@
 	};
 
 	i2s2: i2s at 12D70000 {
-		compatible = "samsung,i2s-v5";
-		reg = <0x12D70000 0x100>;
-		dmas = <&pdma0 12
-			&pdma0 11>;
-		dma-names = "tx", "rx";
 		clocks = <&clock 308>, <&clock 158>;
 		clock-names = "iis", "i2s_opclk0";
 		pinctrl-names = "default";
-- 
1.7.4.4

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

* [PATCH V4 4/4] ARM: dts: Change i2s compatible string on exynos5250
  2013-08-12  9:49 ` Padmavathi Venna
@ 2013-08-12  9:49   ` Padmavathi Venna
  -1 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, alsa-devel, devicetree,
	padma.v, padma.kvr
  Cc: broonie, kgene.kim, tomasz.figa, abrestic

This patch removes quirks from i2s node and change the i2s
compatible names.

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

diff --git a/arch/arm/boot/dts/exynos5.dtsi b/arch/arm/boot/dts/exynos5.dtsi
index aae2fa1..309894e 100644
--- a/arch/arm/boot/dts/exynos5.dtsi
+++ b/arch/arm/boot/dts/exynos5.dtsi
@@ -115,7 +115,7 @@
 	};
 
 	i2s1: i2s@12D60000 {
-		compatible = "samsung,i2s-v5";
+		compatible = "samsung,s3c6410-i2s";
 		reg = <0x12D60000 0x100>;
 		dmas = <&pdma1 12
 			&pdma1 11>;
@@ -123,7 +123,7 @@
 	};
 
 	i2s2: i2s@12D70000 {
-		compatible = "samsung,i2s-v5";
+		compatible = "samsung,s3c6410-i2s";
 		reg = <0x12D70000 0x100>;
 		dmas = <&pdma0 12
 			&pdma0 11>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index f941d52..ac5f5a1 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -405,7 +405,7 @@
 	};
 
 	i2s0: i2s@03830000 {
-		compatible = "samsung,i2s-v5";
+		compatible = "samsung,s5pv210-i2s";
 		dmas = <&pdma0 10
 			&pdma0 9
 			&pdma0 8>;
@@ -414,9 +414,6 @@
 			<&clock_audss EXYNOS_I2S_BUS>,
 			<&clock_audss EXYNOS_SCLK_I2S>;
 		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
-		samsung,supports-6ch;
-		samsung,supports-rstclr;
-		samsung,supports-secdai;
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s0_bus>;
 	};
-- 
1.7.4.4

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

* [PATCH V4 4/4] ARM: dts: Change i2s compatible string on exynos5250
@ 2013-08-12  9:49   ` Padmavathi Venna
  0 siblings, 0 replies; 28+ messages in thread
From: Padmavathi Venna @ 2013-08-12  9:49 UTC (permalink / raw)
  To: linux-arm-kernel

This patch removes quirks from i2s node and change the i2s
compatible names.

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

diff --git a/arch/arm/boot/dts/exynos5.dtsi b/arch/arm/boot/dts/exynos5.dtsi
index aae2fa1..309894e 100644
--- a/arch/arm/boot/dts/exynos5.dtsi
+++ b/arch/arm/boot/dts/exynos5.dtsi
@@ -115,7 +115,7 @@
 	};
 
 	i2s1: i2s at 12D60000 {
-		compatible = "samsung,i2s-v5";
+		compatible = "samsung,s3c6410-i2s";
 		reg = <0x12D60000 0x100>;
 		dmas = <&pdma1 12
 			&pdma1 11>;
@@ -123,7 +123,7 @@
 	};
 
 	i2s2: i2s at 12D70000 {
-		compatible = "samsung,i2s-v5";
+		compatible = "samsung,s3c6410-i2s";
 		reg = <0x12D70000 0x100>;
 		dmas = <&pdma0 12
 			&pdma0 11>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index f941d52..ac5f5a1 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -405,7 +405,7 @@
 	};
 
 	i2s0: i2s at 03830000 {
-		compatible = "samsung,i2s-v5";
+		compatible = "samsung,s5pv210-i2s";
 		dmas = <&pdma0 10
 			&pdma0 9
 			&pdma0 8>;
@@ -414,9 +414,6 @@
 			<&clock_audss EXYNOS_I2S_BUS>,
 			<&clock_audss EXYNOS_SCLK_I2S>;
 		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
-		samsung,supports-6ch;
-		samsung,supports-rstclr;
-		samsung,supports-secdai;
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s0_bus>;
 	};
-- 
1.7.4.4

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

* Re: [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
  2013-08-12  9:49   ` Padmavathi Venna
@ 2013-08-12 12:06     ` Tomasz Figa
  -1 siblings, 0 replies; 28+ messages in thread
From: Tomasz Figa @ 2013-08-12 12:06 UTC (permalink / raw)
  To: Padmavathi Venna
  Cc: linux-samsung-soc, linux-arm-kernel, alsa-devel, devicetree,
	padma.kvr, broonie, kgene.kim, tomasz.figa, abrestic

Hi Padmavathi,

On Monday 12 of August 2013 15:19:51 Padmavathi Venna wrote:
> Samsung has different versions of I2S introduced in different
> platforms. Each version has some new support added for multichannel,
> secondary fifo, s/w reset control and internal mux for rclk src clk.
> Each newly added change has a quirk. So this patch adds all the
> required quirks as driver data and based on compatible string from
> dtsi fetches the quirks.
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  .../devicetree/bindings/sound/samsung-i2s.txt      |   18 ++----
>  sound/soc/samsung/i2s.c                            |   62
> +++++++++++-------- 2 files changed, 42 insertions(+), 38 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> b/Documentation/devicetree/bindings/sound/samsung-i2s.txt index
> 025e66b..25a0024 100644
> --- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> @@ -2,7 +2,11 @@
> 
>  Required SoC Specific Properties:
> 
> -- compatible : "samsung,i2s-v5"
> +- compatible : should be one of the following.
> +   - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
> +   - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
> +     secondary fifo, s/w reset control and internal mux for root clk
> src. +
>  - reg: physical base address of the controller and length of memory
> mapped region.
>  - dmas: list of DMA controller phandle and DMA request line ordered
> pairs. @@ -21,13 +25,6 @@ Required SoC Specific Properties:
> 
>  Optional SoC Specific Properties:
> 
> -- samsung,supports-6ch: If the I2S Primary sound source has 5.1 Channel
> -  support, this flag is enabled.
> -- samsung,supports-rstclr: This flag should be set if I2S software reset
> bit -  control is required. When this flag is set I2S software reset bit
> will be -  enabled or disabled based on need.
> -- samsung,supports-secdai:If I2S block has a secondary FIFO and internal
> DMA, -  then this flag is enabled.
>  - 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.
> @@ -36,7 +33,7 @@ Optional SoC Specific Properties:
>  Example:
> 
>  i2s0: i2s@03830000 {
> -	compatible = "samsung,i2s-v5";
> +	compatible = "samsung,s5pv210-i2s";
>  	reg = <0x03830000 0x100>;
>  	dmas = <&pdma0 10
>  		&pdma0 9
> @@ -46,9 +43,6 @@ i2s0: i2s@03830000 {
>  		<&clock_audss EXYNOS_I2S_BUS>,
>  		<&clock_audss EXYNOS_SCLK_I2S>;
>  	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
> -	samsung,supports-6ch;
> -	samsung,supports-rstclr;
> -	samsung,supports-secdai;
>  	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 47e08dd..1671d9b 100644
> --- a/sound/soc/samsung/i2s.c
> +++ b/sound/soc/samsung/i2s.c
> @@ -40,6 +40,7 @@ enum samsung_dai_type {
> 
>  struct samsung_i2s_dai_data {
>  	int dai_type;
> +	u32 quirks;
>  };
> 
>  struct i2s_dai {
> @@ -1032,18 +1033,18 @@ static struct i2s_dai *i2s_alloc_dai(struct
> platform_device *pdev, bool sec)
> 
>  static const struct of_device_id exynos_i2s_match[];
> 
> -static inline int samsung_i2s_get_driver_data(struct platform_device
> *pdev) +static inline const struct samsung_i2s_dai_data
> *samsung_i2s_get_driver_data( +						struct platform_device 
*pdev)
>  {
>  #ifdef CONFIG_OF
> -	struct samsung_i2s_dai_data *data;
>  	if (pdev->dev.of_node) {
>  		const struct of_device_id *match;
>  		match = of_match_node(exynos_i2s_match, pdev->dev.of_node);
> -		data = (struct samsung_i2s_dai_data *) match->data;
> -		return data->dai_type;
> +		return match->data;
>  	} else
>  #endif
> -		return platform_get_device_id(pdev)->driver_data;
> +		return (struct samsung_i2s_dai_data *)
> +				platform_get_device_id(pdev)->driver_data;
>  }
> 
>  #ifdef CONFIG_PM_RUNTIME
> @@ -1074,13 +1075,13 @@ static int samsung_i2s_probe(struct
> platform_device *pdev) struct resource *res;
>  	u32 regs_base, quirks = 0, idma_addr = 0;
>  	struct device_node *np = pdev->dev.of_node;
> -	enum samsung_dai_type samsung_dai_type;
> +	const struct samsung_i2s_dai_data *i2s_dai_data;
>  	int ret = 0;
> 
>  	/* Call during Seconday interface registration */
> -	samsung_dai_type = samsung_i2s_get_driver_data(pdev);
> +	i2s_dai_data = samsung_i2s_get_driver_data(pdev);
> 
> -	if (samsung_dai_type == TYPE_SEC) {
> +	if (i2s_dai_data->dai_type == TYPE_SEC) {
>  		sec_dai = dev_get_drvdata(&pdev->dev);
>  		if (!sec_dai) {
>  			dev_err(&pdev->dev, "Unable to get drvdata\n");
> @@ -1129,15 +1130,7 @@ static int samsung_i2s_probe(struct
> platform_device *pdev) idma_addr = i2s_cfg->idma_addr;
>  		}
>  	} else {
> -		if (of_find_property(np, "samsung,supports-6ch", NULL))
> -			quirks |= QUIRK_PRI_6CHAN;
> -
> -		if (of_find_property(np, "samsung,supports-secdai", NULL))
> -			quirks |= QUIRK_SEC_DAI;
> -
> -		if (of_find_property(np, "samsung,supports-rstclr", NULL))
> -			quirks |= QUIRK_NEED_RSTCLR;
> -
> +		quirks = i2s_dai_data->quirks;
>  		if (of_property_read_u32(np, "samsung,idma-addr",
>  					 &idma_addr)) {
>  			if (quirks & QUIRK_SEC_DAI) {
> @@ -1250,27 +1243,44 @@ static int samsung_i2s_remove(struct
> platform_device *pdev) return 0;
>  }
> 
> +static const struct samsung_i2s_dai_data i2sv3_dai_type = {
> +	.dai_type = TYPE_PRI,
> +	.quirks = QUIRK_NO_MUXPSR,
> +};
> +
> +static const struct samsung_i2s_dai_data i2sv5_dai_type = {
> +	.dai_type = TYPE_PRI,
> +	.quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR,
> +};
> +
> +static const struct samsung_i2s_dai_data samsung_dai_type_pri = {
> +	.dai_type = TYPE_PRI,
> +};
> +
> +static const struct samsung_i2s_dai_data samsung_dai_type_sec = {
> +	.dai_type = TYPE_SEC,
> +};
> +
>  static struct platform_device_id samsung_i2s_driver_ids[] = {
>  	{
>  		.name           = "samsung-i2s",
> -		.driver_data	= TYPE_PRI,
> +		.driver_data    = (kernel_ulong_t)&samsung_dai_type_pri,
>  	}, {
>  		.name           = "samsung-i2s-sec",
> -		.driver_data	= TYPE_SEC,
> +		.driver_data    = (kernel_ulong_t)&samsung_dai_type_sec,
>  	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(platform, samsung_i2s_driver_ids);
> 
>  #ifdef CONFIG_OF
> -static struct samsung_i2s_dai_data samsung_i2s_dai_data_array[] = {
> -	[TYPE_PRI] = { TYPE_PRI },
> -	[TYPE_SEC] = { TYPE_SEC },
> -};
> -
>  static const struct of_device_id exynos_i2s_match[] = {
> -	{ .compatible = "samsung,i2s-v5",
> -	  .data = &samsung_i2s_dai_data_array[TYPE_PRI],
> +	{
> +		.compatible = "samsung,s3c6410-i2s",
> +		.data = &i2sv3_dai_type,
> +	}, {
> +		.compatible = "samsung,s5pv210-i2s",
> +		.data = &i2sv5_dai_type,

There is still a hole in this series, between patch 1/4 and 4/4, where 
existing platforms are broken. This will break bisects.

I have commented on this in previous version of this series and suggested 
two possible solutions, one preferred and one low-effort.

Otherwise looks good.

Best regards,
Tomasz

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

* [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
@ 2013-08-12 12:06     ` Tomasz Figa
  0 siblings, 0 replies; 28+ messages in thread
From: Tomasz Figa @ 2013-08-12 12:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Padmavathi,

On Monday 12 of August 2013 15:19:51 Padmavathi Venna wrote:
> Samsung has different versions of I2S introduced in different
> platforms. Each version has some new support added for multichannel,
> secondary fifo, s/w reset control and internal mux for rclk src clk.
> Each newly added change has a quirk. So this patch adds all the
> required quirks as driver data and based on compatible string from
> dtsi fetches the quirks.
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> ---
>  .../devicetree/bindings/sound/samsung-i2s.txt      |   18 ++----
>  sound/soc/samsung/i2s.c                            |   62
> +++++++++++-------- 2 files changed, 42 insertions(+), 38 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> b/Documentation/devicetree/bindings/sound/samsung-i2s.txt index
> 025e66b..25a0024 100644
> --- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> @@ -2,7 +2,11 @@
> 
>  Required SoC Specific Properties:
> 
> -- compatible : "samsung,i2s-v5"
> +- compatible : should be one of the following.
> +   - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
> +   - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
> +     secondary fifo, s/w reset control and internal mux for root clk
> src. +
>  - reg: physical base address of the controller and length of memory
> mapped region.
>  - dmas: list of DMA controller phandle and DMA request line ordered
> pairs. @@ -21,13 +25,6 @@ Required SoC Specific Properties:
> 
>  Optional SoC Specific Properties:
> 
> -- samsung,supports-6ch: If the I2S Primary sound source has 5.1 Channel
> -  support, this flag is enabled.
> -- samsung,supports-rstclr: This flag should be set if I2S software reset
> bit -  control is required. When this flag is set I2S software reset bit
> will be -  enabled or disabled based on need.
> -- samsung,supports-secdai:If I2S block has a secondary FIFO and internal
> DMA, -  then this flag is enabled.
>  - 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.
> @@ -36,7 +33,7 @@ Optional SoC Specific Properties:
>  Example:
> 
>  i2s0: i2s at 03830000 {
> -	compatible = "samsung,i2s-v5";
> +	compatible = "samsung,s5pv210-i2s";
>  	reg = <0x03830000 0x100>;
>  	dmas = <&pdma0 10
>  		&pdma0 9
> @@ -46,9 +43,6 @@ i2s0: i2s at 03830000 {
>  		<&clock_audss EXYNOS_I2S_BUS>,
>  		<&clock_audss EXYNOS_SCLK_I2S>;
>  	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
> -	samsung,supports-6ch;
> -	samsung,supports-rstclr;
> -	samsung,supports-secdai;
>  	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 47e08dd..1671d9b 100644
> --- a/sound/soc/samsung/i2s.c
> +++ b/sound/soc/samsung/i2s.c
> @@ -40,6 +40,7 @@ enum samsung_dai_type {
> 
>  struct samsung_i2s_dai_data {
>  	int dai_type;
> +	u32 quirks;
>  };
> 
>  struct i2s_dai {
> @@ -1032,18 +1033,18 @@ static struct i2s_dai *i2s_alloc_dai(struct
> platform_device *pdev, bool sec)
> 
>  static const struct of_device_id exynos_i2s_match[];
> 
> -static inline int samsung_i2s_get_driver_data(struct platform_device
> *pdev) +static inline const struct samsung_i2s_dai_data
> *samsung_i2s_get_driver_data( +						struct platform_device 
*pdev)
>  {
>  #ifdef CONFIG_OF
> -	struct samsung_i2s_dai_data *data;
>  	if (pdev->dev.of_node) {
>  		const struct of_device_id *match;
>  		match = of_match_node(exynos_i2s_match, pdev->dev.of_node);
> -		data = (struct samsung_i2s_dai_data *) match->data;
> -		return data->dai_type;
> +		return match->data;
>  	} else
>  #endif
> -		return platform_get_device_id(pdev)->driver_data;
> +		return (struct samsung_i2s_dai_data *)
> +				platform_get_device_id(pdev)->driver_data;
>  }
> 
>  #ifdef CONFIG_PM_RUNTIME
> @@ -1074,13 +1075,13 @@ static int samsung_i2s_probe(struct
> platform_device *pdev) struct resource *res;
>  	u32 regs_base, quirks = 0, idma_addr = 0;
>  	struct device_node *np = pdev->dev.of_node;
> -	enum samsung_dai_type samsung_dai_type;
> +	const struct samsung_i2s_dai_data *i2s_dai_data;
>  	int ret = 0;
> 
>  	/* Call during Seconday interface registration */
> -	samsung_dai_type = samsung_i2s_get_driver_data(pdev);
> +	i2s_dai_data = samsung_i2s_get_driver_data(pdev);
> 
> -	if (samsung_dai_type == TYPE_SEC) {
> +	if (i2s_dai_data->dai_type == TYPE_SEC) {
>  		sec_dai = dev_get_drvdata(&pdev->dev);
>  		if (!sec_dai) {
>  			dev_err(&pdev->dev, "Unable to get drvdata\n");
> @@ -1129,15 +1130,7 @@ static int samsung_i2s_probe(struct
> platform_device *pdev) idma_addr = i2s_cfg->idma_addr;
>  		}
>  	} else {
> -		if (of_find_property(np, "samsung,supports-6ch", NULL))
> -			quirks |= QUIRK_PRI_6CHAN;
> -
> -		if (of_find_property(np, "samsung,supports-secdai", NULL))
> -			quirks |= QUIRK_SEC_DAI;
> -
> -		if (of_find_property(np, "samsung,supports-rstclr", NULL))
> -			quirks |= QUIRK_NEED_RSTCLR;
> -
> +		quirks = i2s_dai_data->quirks;
>  		if (of_property_read_u32(np, "samsung,idma-addr",
>  					 &idma_addr)) {
>  			if (quirks & QUIRK_SEC_DAI) {
> @@ -1250,27 +1243,44 @@ static int samsung_i2s_remove(struct
> platform_device *pdev) return 0;
>  }
> 
> +static const struct samsung_i2s_dai_data i2sv3_dai_type = {
> +	.dai_type = TYPE_PRI,
> +	.quirks = QUIRK_NO_MUXPSR,
> +};
> +
> +static const struct samsung_i2s_dai_data i2sv5_dai_type = {
> +	.dai_type = TYPE_PRI,
> +	.quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR,
> +};
> +
> +static const struct samsung_i2s_dai_data samsung_dai_type_pri = {
> +	.dai_type = TYPE_PRI,
> +};
> +
> +static const struct samsung_i2s_dai_data samsung_dai_type_sec = {
> +	.dai_type = TYPE_SEC,
> +};
> +
>  static struct platform_device_id samsung_i2s_driver_ids[] = {
>  	{
>  		.name           = "samsung-i2s",
> -		.driver_data	= TYPE_PRI,
> +		.driver_data    = (kernel_ulong_t)&samsung_dai_type_pri,
>  	}, {
>  		.name           = "samsung-i2s-sec",
> -		.driver_data	= TYPE_SEC,
> +		.driver_data    = (kernel_ulong_t)&samsung_dai_type_sec,
>  	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(platform, samsung_i2s_driver_ids);
> 
>  #ifdef CONFIG_OF
> -static struct samsung_i2s_dai_data samsung_i2s_dai_data_array[] = {
> -	[TYPE_PRI] = { TYPE_PRI },
> -	[TYPE_SEC] = { TYPE_SEC },
> -};
> -
>  static const struct of_device_id exynos_i2s_match[] = {
> -	{ .compatible = "samsung,i2s-v5",
> -	  .data = &samsung_i2s_dai_data_array[TYPE_PRI],
> +	{
> +		.compatible = "samsung,s3c6410-i2s",
> +		.data = &i2sv3_dai_type,
> +	}, {
> +		.compatible = "samsung,s5pv210-i2s",
> +		.data = &i2sv5_dai_type,

There is still a hole in this series, between patch 1/4 and 4/4, where 
existing platforms are broken. This will break bisects.

I have commented on this in previous version of this series and suggested 
two possible solutions, one preferred and one low-effort.

Otherwise looks good.

Best regards,
Tomasz

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

* Re: [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
  2013-08-12  9:49   ` Padmavathi Venna
@ 2013-08-12 22:57     ` Stephen Warren
  -1 siblings, 0 replies; 28+ messages in thread
From: Stephen Warren @ 2013-08-12 22:57 UTC (permalink / raw)
  To: Padmavathi Venna
  Cc: linux-samsung-soc, linux-arm-kernel, alsa-devel, devicetree,
	padma.kvr, broonie, kgene.kim, tomasz.figa, abrestic,
	Mark Rutland, Ian Campbell, Pawel Moll, Rob Herring, Kumar Gala

(CCing DT maintainers...)

On 08/12/2013 03:49 AM, Padmavathi Venna wrote:
> Samsung has different versions of I2S introduced in different
> platforms. Each version has some new support added for multichannel,
> secondary fifo, s/w reset control and internal mux for rclk src clk.
> Each newly added change has a quirk. So this patch adds all the
> required quirks as driver data and based on compatible string from
> dtsi fetches the quirks.

> diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> index 025e66b..25a0024 100644
> --- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> @@ -2,7 +2,11 @@
>  
>  Required SoC Specific Properties:
>  
> -- compatible : "samsung,i2s-v5"
> +- compatible : should be one of the following.
> +   - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
> +   - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
> +     secondary fifo, s/w reset control and internal mux for root clk src.

Those descriptions seem a little odd. If I have an SoC that isn't
s5pv210, yet supports "8/16/24bit multichannel(5.1) I2S with secondary
fifo, s/w reset control and internal mux for root clk src", will
compatible="samsung,s5pv210-i2s" work for my HW?

I wonder if you should instead include the IP block version in the
compatible value?

Alternatively, perhaps simply removing the description of the
capabilities of each SoC would make the binding document more typical.

Although all that said, given the semantic meaning of compatible; to
describe which specific SW-visible interface is available (which include
the feature-set), I feel my comments are a little odd:-)

>  - reg: physical base address of the controller and length of memory mapped
>    region.
>  - dmas: list of DMA controller phandle and DMA request line ordered pairs.
> @@ -21,13 +25,6 @@ Required SoC Specific Properties:
>  
>  Optional SoC Specific Properties:
>  
> -- samsung,supports-6ch: If the I2S Primary sound source has 5.1 Channel
> -  support, this flag is enabled.
> -- samsung,supports-rstclr: This flag should be set if I2S software reset bit
> -  control is required. When this flag is set I2S software reset bit will be
> -  enabled or disabled based on need.
> -- samsung,supports-secdai:If I2S block has a secondary FIFO and internal DMA,
> -  then this flag is enabled.
>  - samsung,idma-addr: Internal DMA register base address of the audio
>    sub system(used in secondary sound source).

Is it likely that a driver that fully implements those properties can
run on future HW without modification, perhaps adding some extra
properties to enable any new features?

In other words, I don't think we have an answer to the question: Should
differences between similar HW blocks be encoded into DT properties, or
should the driver encode them into some table, and look them up from
compatible value?

This patch changes between those two options. I'm not sure if there's
consensus that one option is better than the other, and hence whether
this patch is actually necessary or useful.

(although I dare say that at least samsung,supports-rstclr should be
modified to use the new reset controller bindings)

>  - pinctrl-0: Should specify pin control groups used for this controller.
> @@ -36,7 +33,7 @@ Optional SoC Specific Properties:
>  Example:
>  
>  i2s0: i2s@03830000 {
> -	compatible = "samsung,i2s-v5";
> +	compatible = "samsung,s5pv210-i2s";
>  	reg = <0x03830000 0x100>;
>  	dmas = <&pdma0 10
>  		&pdma0 9
> @@ -46,9 +43,6 @@ i2s0: i2s@03830000 {
>  		<&clock_audss EXYNOS_I2S_BUS>,
>  		<&clock_audss EXYNOS_SCLK_I2S>;
>  	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
> -	samsung,supports-6ch;
> -	samsung,supports-rstclr;
> -	samsung,supports-secdai;
>  	samsung,idma-addr = <0x03000000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&i2s0_bus>;

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

* [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
@ 2013-08-12 22:57     ` Stephen Warren
  0 siblings, 0 replies; 28+ messages in thread
From: Stephen Warren @ 2013-08-12 22:57 UTC (permalink / raw)
  To: linux-arm-kernel

(CCing DT maintainers...)

On 08/12/2013 03:49 AM, Padmavathi Venna wrote:
> Samsung has different versions of I2S introduced in different
> platforms. Each version has some new support added for multichannel,
> secondary fifo, s/w reset control and internal mux for rclk src clk.
> Each newly added change has a quirk. So this patch adds all the
> required quirks as driver data and based on compatible string from
> dtsi fetches the quirks.

> diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> index 025e66b..25a0024 100644
> --- a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
> @@ -2,7 +2,11 @@
>  
>  Required SoC Specific Properties:
>  
> -- compatible : "samsung,i2s-v5"
> +- compatible : should be one of the following.
> +   - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
> +   - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
> +     secondary fifo, s/w reset control and internal mux for root clk src.

Those descriptions seem a little odd. If I have an SoC that isn't
s5pv210, yet supports "8/16/24bit multichannel(5.1) I2S with secondary
fifo, s/w reset control and internal mux for root clk src", will
compatible="samsung,s5pv210-i2s" work for my HW?

I wonder if you should instead include the IP block version in the
compatible value?

Alternatively, perhaps simply removing the description of the
capabilities of each SoC would make the binding document more typical.

Although all that said, given the semantic meaning of compatible; to
describe which specific SW-visible interface is available (which include
the feature-set), I feel my comments are a little odd:-)

>  - reg: physical base address of the controller and length of memory mapped
>    region.
>  - dmas: list of DMA controller phandle and DMA request line ordered pairs.
> @@ -21,13 +25,6 @@ Required SoC Specific Properties:
>  
>  Optional SoC Specific Properties:
>  
> -- samsung,supports-6ch: If the I2S Primary sound source has 5.1 Channel
> -  support, this flag is enabled.
> -- samsung,supports-rstclr: This flag should be set if I2S software reset bit
> -  control is required. When this flag is set I2S software reset bit will be
> -  enabled or disabled based on need.
> -- samsung,supports-secdai:If I2S block has a secondary FIFO and internal DMA,
> -  then this flag is enabled.
>  - samsung,idma-addr: Internal DMA register base address of the audio
>    sub system(used in secondary sound source).

Is it likely that a driver that fully implements those properties can
run on future HW without modification, perhaps adding some extra
properties to enable any new features?

In other words, I don't think we have an answer to the question: Should
differences between similar HW blocks be encoded into DT properties, or
should the driver encode them into some table, and look them up from
compatible value?

This patch changes between those two options. I'm not sure if there's
consensus that one option is better than the other, and hence whether
this patch is actually necessary or useful.

(although I dare say that at least samsung,supports-rstclr should be
modified to use the new reset controller bindings)

>  - pinctrl-0: Should specify pin control groups used for this controller.
> @@ -36,7 +33,7 @@ Optional SoC Specific Properties:
>  Example:
>  
>  i2s0: i2s at 03830000 {
> -	compatible = "samsung,i2s-v5";
> +	compatible = "samsung,s5pv210-i2s";
>  	reg = <0x03830000 0x100>;
>  	dmas = <&pdma0 10
>  		&pdma0 9
> @@ -46,9 +43,6 @@ i2s0: i2s at 03830000 {
>  		<&clock_audss EXYNOS_I2S_BUS>,
>  		<&clock_audss EXYNOS_SCLK_I2S>;
>  	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
> -	samsung,supports-6ch;
> -	samsung,supports-rstclr;
> -	samsung,supports-secdai;
>  	samsung,idma-addr = <0x03000000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&i2s0_bus>;

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

* Re: [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
  2013-08-12 22:57     ` Stephen Warren
@ 2013-08-12 23:13       ` Mark Brown
  -1 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2013-08-12 23:13 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Padmavathi Venna, linux-samsung-soc, linux-arm-kernel,
	alsa-devel, devicetree, padma.kvr, kgene.kim, tomasz.figa,
	abrestic, Mark Rutland, Ian Campbell, Pawel Moll, Rob Herring,
	Kumar Gala

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

On Mon, Aug 12, 2013 at 04:57:53PM -0600, Stephen Warren wrote:
> On 08/12/2013 03:49 AM, Padmavathi Venna wrote:

> > +- compatible : should be one of the following.
> > +   - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
> > +   - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
> > +     secondary fifo, s/w reset control and internal mux for root clk src.

> Those descriptions seem a little odd. If I have an SoC that isn't
> s5pv210, yet supports "8/16/24bit multichannel(5.1) I2S with secondary
> fifo, s/w reset control and internal mux for root clk src", will
> compatible="samsung,s5pv210-i2s" work for my HW?

> I wonder if you should instead include the IP block version in the
> compatible value?

We've been round this loop several times, I'd prefer the IP block
versions too but they're at best patchily documented and so as a general
policy the Samsung bindings use the name of the SoC an IP first appeared
in as the version.

> In other words, I don't think we have an answer to the question: Should
> differences between similar HW blocks be encoded into DT properties, or
> should the driver encode them into some table, and look them up from
> compatible value?

For usability it seems better to just be able to say which IP you've
got, this also makes it easier to implement support for new IP features
later on without having to go back and add new properties which would be
sad.

> (although I dare say that at least samsung,supports-rstclr should be
> modified to use the new reset controller bindings)

Really?  That doesn't seem terribly sane - I had thought that was for
bodging resets on the side of things that don't normally have them or
need board specific logic.  Also note that this is actually a magic
register write done to reset the IP on some specific IPs.

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

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

* [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
@ 2013-08-12 23:13       ` Mark Brown
  0 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2013-08-12 23:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 12, 2013 at 04:57:53PM -0600, Stephen Warren wrote:
> On 08/12/2013 03:49 AM, Padmavathi Venna wrote:

> > +- compatible : should be one of the following.
> > +   - samsung,s3c6410-i2s: for 8/16/24bit stereo I2S.
> > +   - samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with
> > +     secondary fifo, s/w reset control and internal mux for root clk src.

> Those descriptions seem a little odd. If I have an SoC that isn't
> s5pv210, yet supports "8/16/24bit multichannel(5.1) I2S with secondary
> fifo, s/w reset control and internal mux for root clk src", will
> compatible="samsung,s5pv210-i2s" work for my HW?

> I wonder if you should instead include the IP block version in the
> compatible value?

We've been round this loop several times, I'd prefer the IP block
versions too but they're at best patchily documented and so as a general
policy the Samsung bindings use the name of the SoC an IP first appeared
in as the version.

> In other words, I don't think we have an answer to the question: Should
> differences between similar HW blocks be encoded into DT properties, or
> should the driver encode them into some table, and look them up from
> compatible value?

For usability it seems better to just be able to say which IP you've
got, this also makes it easier to implement support for new IP features
later on without having to go back and add new properties which would be
sad.

> (although I dare say that at least samsung,supports-rstclr should be
> modified to use the new reset controller bindings)

Really?  That doesn't seem terribly sane - I had thought that was for
bodging resets on the side of things that don't normally have them or
need board specific logic.  Also note that this is actually a magic
register write done to reset the IP on some specific IPs.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130813/390236c3/attachment.sig>

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

* Re: [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
  2013-08-12 23:13       ` Mark Brown
@ 2013-08-12 23:18         ` Stephen Warren
  -1 siblings, 0 replies; 28+ messages in thread
From: Stephen Warren @ 2013-08-12 23:18 UTC (permalink / raw)
  To: Mark Brown
  Cc: Padmavathi Venna, linux-samsung-soc, linux-arm-kernel,
	alsa-devel, devicetree, padma.kvr, kgene.kim, tomasz.figa,
	abrestic, Mark Rutland, Ian Campbell, Pawel Moll, Rob Herring,
	Kumar Gala

On 08/12/2013 05:13 PM, Mark Brown wrote:
> On Mon, Aug 12, 2013 at 04:57:53PM -0600, Stephen Warren wrote:
>> On 08/12/2013 03:49 AM, Padmavathi Venna wrote:
> 
>>> +- compatible : should be one of the following. +   -
>>> samsung,s3c6410-i2s: for 8/16/24bit stereo I2S. +   -
>>> samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with 
>>> +     secondary fifo, s/w reset control and internal mux for
>>> root clk src.
> 
>> Those descriptions seem a little odd. If I have an SoC that
>> isn't s5pv210, yet supports "8/16/24bit multichannel(5.1) I2S
>> with secondary fifo, s/w reset control and internal mux for root
>> clk src", will compatible="samsung,s5pv210-i2s" work for my HW?
> 
>> I wonder if you should instead include the IP block version in
>> the compatible value?
> 
> We've been round this loop several times, I'd prefer the IP block 
> versions too but they're at best patchily documented and so as a
> general policy the Samsung bindings use the name of the SoC an IP
> first appeared in as the version.
> 
>> In other words, I don't think we have an answer to the question:
>> Should differences between similar HW blocks be encoded into DT
>> properties, or should the driver encode them into some table, and
>> look them up from compatible value?
> 
> For usability it seems better to just be able to say which IP
> you've got, this also makes it easier to implement support for new
> IP features later on without having to go back and add new
> properties which would be sad.

That seems quite reasonable, but I don't think everyone involved in DT
has come out and agreed on that. I'm quite happy with the approach of
looking up everything based on compatible.

>> (although I dare say that at least samsung,supports-rstclr should
>> be modified to use the new reset controller bindings)
> 
> Really?  That doesn't seem terribly sane - I had thought that was
> for bodging resets on the side of things that don't normally have
> them or need board specific logic.  Also note that this is actually
> a magic register write done to reset the IP on some specific IPs.

I believe that's exactly what the reset subsystem and associated DT
bindings were designed for.

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

* [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
@ 2013-08-12 23:18         ` Stephen Warren
  0 siblings, 0 replies; 28+ messages in thread
From: Stephen Warren @ 2013-08-12 23:18 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/12/2013 05:13 PM, Mark Brown wrote:
> On Mon, Aug 12, 2013 at 04:57:53PM -0600, Stephen Warren wrote:
>> On 08/12/2013 03:49 AM, Padmavathi Venna wrote:
> 
>>> +- compatible : should be one of the following. +   -
>>> samsung,s3c6410-i2s: for 8/16/24bit stereo I2S. +   -
>>> samsung,s5pv210-i2s: for 8/16/24bit multichannel(5.1) I2S with 
>>> +     secondary fifo, s/w reset control and internal mux for
>>> root clk src.
> 
>> Those descriptions seem a little odd. If I have an SoC that
>> isn't s5pv210, yet supports "8/16/24bit multichannel(5.1) I2S
>> with secondary fifo, s/w reset control and internal mux for root
>> clk src", will compatible="samsung,s5pv210-i2s" work for my HW?
> 
>> I wonder if you should instead include the IP block version in
>> the compatible value?
> 
> We've been round this loop several times, I'd prefer the IP block 
> versions too but they're at best patchily documented and so as a
> general policy the Samsung bindings use the name of the SoC an IP
> first appeared in as the version.
> 
>> In other words, I don't think we have an answer to the question:
>> Should differences between similar HW blocks be encoded into DT
>> properties, or should the driver encode them into some table, and
>> look them up from compatible value?
> 
> For usability it seems better to just be able to say which IP
> you've got, this also makes it easier to implement support for new
> IP features later on without having to go back and add new
> properties which would be sad.

That seems quite reasonable, but I don't think everyone involved in DT
has come out and agreed on that. I'm quite happy with the approach of
looking up everything based on compatible.

>> (although I dare say that at least samsung,supports-rstclr should
>> be modified to use the new reset controller bindings)
> 
> Really?  That doesn't seem terribly sane - I had thought that was
> for bodging resets on the side of things that don't normally have
> them or need board specific logic.  Also note that this is actually
> a magic register write done to reset the IP on some specific IPs.

I believe that's exactly what the reset subsystem and associated DT
bindings were designed for.

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

* Re: [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
  2013-08-12 23:18         ` Stephen Warren
@ 2013-08-12 23:46           ` Mark Brown
  -1 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2013-08-12 23:46 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Mark Rutland, devicetree, alsa-devel, linux-samsung-soc,
	Ian Campbell, Pawel Moll, Padmavathi Venna, abrestic,
	tomasz.figa, Rob Herring, kgene.kim, Kumar Gala, padma.kvr,
	linux-arm-kernel


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

On Mon, Aug 12, 2013 at 05:18:34PM -0600, Stephen Warren wrote:
> On 08/12/2013 05:13 PM, Mark Brown wrote:

> >> (although I dare say that at least samsung,supports-rstclr should
> >> be modified to use the new reset controller bindings)

> > Really?  That doesn't seem terribly sane - I had thought that was
> > for bodging resets on the side of things that don't normally have
> > them or need board specific logic.  Also note that this is actually
> > a magic register write done to reset the IP on some specific IPs.

> I believe that's exactly what the reset subsystem and associated DT
> bindings were designed for.

That seems...  interesting.  It seems like this is fairly core device
functionality I'd expect the driver to just be able to understand;  the
main thing this property was doing was deciding if the reset was needed.
I'm not sure I see the benefit here?

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

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



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

* [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
@ 2013-08-12 23:46           ` Mark Brown
  0 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2013-08-12 23:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 12, 2013 at 05:18:34PM -0600, Stephen Warren wrote:
> On 08/12/2013 05:13 PM, Mark Brown wrote:

> >> (although I dare say that at least samsung,supports-rstclr should
> >> be modified to use the new reset controller bindings)

> > Really?  That doesn't seem terribly sane - I had thought that was
> > for bodging resets on the side of things that don't normally have
> > them or need board specific logic.  Also note that this is actually
> > a magic register write done to reset the IP on some specific IPs.

> I believe that's exactly what the reset subsystem and associated DT
> bindings were designed for.

That seems...  interesting.  It seems like this is fairly core device
functionality I'd expect the driver to just be able to understand;  the
main thing this property was doing was deciding if the reset was needed.
I'm not sure I see the benefit here?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130813/90b207b2/attachment.sig>

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

* Re: [PATCH V4 0/4] Add i2s support on smdk5420
  2013-08-12  9:49 ` Padmavathi Venna
@ 2013-08-13 12:44   ` Mark Brown
  -1 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2013-08-13 12:44 UTC (permalink / raw)
  To: Padmavathi Venna
  Cc: devicetree, alsa-devel, linux-samsung-soc, abrestic, tomasz.figa,
	kgene.kim, padma.kvr, linux-arm-kernel


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

On Mon, Aug 12, 2013 at 03:19:50PM +0530, Padmavathi Venna wrote:
> Samsung has different versions of I2S introduced in different
> platforms. Each version has some new support added for multichannel,
> secondary fifo, s/w reset control, internal mux for rclk src clk and
> tdm support. Each newly added change has a quirk. So this patch adds
> all the required quirks as driver data and based on compatible string
> from dtsi fetches the quirks. This also adds i2s support on exynos5420.

Applied all these, thanks.

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

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



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

* [PATCH V4 0/4] Add i2s support on smdk5420
@ 2013-08-13 12:44   ` Mark Brown
  0 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2013-08-13 12:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 12, 2013 at 03:19:50PM +0530, Padmavathi Venna wrote:
> Samsung has different versions of I2S introduced in different
> platforms. Each version has some new support added for multichannel,
> secondary fifo, s/w reset control, internal mux for rclk src clk and
> tdm support. Each newly added change has a quirk. So this patch adds
> all the required quirks as driver data and based on compatible string
> from dtsi fetches the quirks. This also adds i2s support on exynos5420.

Applied all these, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130813/2b18da26/attachment.sig>

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

* Re: [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
  2013-08-12 23:46           ` Mark Brown
@ 2013-08-13 15:42             ` Stephen Warren
  -1 siblings, 0 replies; 28+ messages in thread
From: Stephen Warren @ 2013-08-13 15:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: Padmavathi Venna, linux-samsung-soc, linux-arm-kernel,
	alsa-devel, devicetree, padma.kvr, kgene.kim, tomasz.figa,
	abrestic, Mark Rutland, Ian Campbell, Pawel Moll, Rob Herring,
	Kumar Gala

On 08/12/2013 05:46 PM, Mark Brown wrote:
> On Mon, Aug 12, 2013 at 05:18:34PM -0600, Stephen Warren wrote:
>> On 08/12/2013 05:13 PM, Mark Brown wrote:
> 
>>>> (although I dare say that at least samsung,supports-rstclr
>>>> should be modified to use the new reset controller bindings)
> 
>>> Really?  That doesn't seem terribly sane - I had thought that
>>> was for bodging resets on the side of things that don't
>>> normally have them or need board specific logic.  Also note
>>> that this is actually a magic register write done to reset the
>>> IP on some specific IPs.
> 
>> I believe that's exactly what the reset subsystem and associated
>> DT bindings were designed for.
> 
> That seems...  interesting.  It seems like this is fairly core
> device functionality I'd expect the driver to just be able to
> understand;  the main thing this property was doing was deciding if
> the reset was needed. I'm not sure I see the benefit here?

Sorry, I'm confused here. In this case, the reset is something
internal to the IP module, not sourced by an external reset
controller, so there's no need to involve the reset controller
bindings or subsystem.

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

* [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S
@ 2013-08-13 15:42             ` Stephen Warren
  0 siblings, 0 replies; 28+ messages in thread
From: Stephen Warren @ 2013-08-13 15:42 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/12/2013 05:46 PM, Mark Brown wrote:
> On Mon, Aug 12, 2013 at 05:18:34PM -0600, Stephen Warren wrote:
>> On 08/12/2013 05:13 PM, Mark Brown wrote:
> 
>>>> (although I dare say that at least samsung,supports-rstclr
>>>> should be modified to use the new reset controller bindings)
> 
>>> Really?  That doesn't seem terribly sane - I had thought that
>>> was for bodging resets on the side of things that don't
>>> normally have them or need board specific logic.  Also note
>>> that this is actually a magic register write done to reset the
>>> IP on some specific IPs.
> 
>> I believe that's exactly what the reset subsystem and associated
>> DT bindings were designed for.
> 
> That seems...  interesting.  It seems like this is fairly core
> device functionality I'd expect the driver to just be able to
> understand;  the main thing this property was doing was deciding if
> the reset was needed. I'm not sure I see the benefit here?

Sorry, I'm confused here. In this case, the reset is something
internal to the IP module, not sourced by an external reset
controller, so there's no need to involve the reset controller
bindings or subsystem.

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

* Re: [PATCH V4 0/4] Add i2s support on smdk5420
  2013-08-13 12:44   ` Mark Brown
@ 2013-08-14  8:25     ` Tomasz Figa
  -1 siblings, 0 replies; 28+ messages in thread
From: Tomasz Figa @ 2013-08-14  8:25 UTC (permalink / raw)
  To: Mark Brown
  Cc: Padmavathi Venna, linux-samsung-soc, linux-arm-kernel,
	alsa-devel, devicetree, padma.kvr, kgene.kim, abrestic

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

On Tuesday 13 of August 2013 13:44:40 Mark Brown wrote:
> On Mon, Aug 12, 2013 at 03:19:50PM +0530, Padmavathi Venna wrote:
> > Samsung has different versions of I2S introduced in different
> > platforms. Each version has some new support added for multichannel,
> > secondary fifo, s/w reset control, internal mux for rclk src clk and
> > tdm support. Each newly added change has a quirk. So this patch adds
> > all the required quirks as driver data and based on compatible string
> > from dtsi fetches the quirks. This also adds i2s support on
> > exynos5420.
> 
> Applied all these, thanks.

Hmm, this series looks good to me too, except one thing. What about the 
bisection breakage introduced by the hole between patches 1/4 and 4/4 
(after the list of supported compatible changes in the driver and before 
respective dts files are updated)?

Best regards,
Tomasz

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

* [PATCH V4 0/4] Add i2s support on smdk5420
@ 2013-08-14  8:25     ` Tomasz Figa
  0 siblings, 0 replies; 28+ messages in thread
From: Tomasz Figa @ 2013-08-14  8:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 13 of August 2013 13:44:40 Mark Brown wrote:
> On Mon, Aug 12, 2013 at 03:19:50PM +0530, Padmavathi Venna wrote:
> > Samsung has different versions of I2S introduced in different
> > platforms. Each version has some new support added for multichannel,
> > secondary fifo, s/w reset control, internal mux for rclk src clk and
> > tdm support. Each newly added change has a quirk. So this patch adds
> > all the required quirks as driver data and based on compatible string
> > from dtsi fetches the quirks. This also adds i2s support on
> > exynos5420.
> 
> Applied all these, thanks.

Hmm, this series looks good to me too, except one thing. What about the 
bisection breakage introduced by the hole between patches 1/4 and 4/4 
(after the list of supported compatible changes in the driver and before 
respective dts files are updated)?

Best regards,
Tomasz
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130814/8e85ba14/attachment.sig>

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

* Re: [PATCH V4 0/4] Add i2s support on smdk5420
  2013-08-14  8:25     ` Tomasz Figa
@ 2013-08-14 10:03       ` Mark Brown
  -1 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2013-08-14 10:03 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Padmavathi Venna, linux-samsung-soc, linux-arm-kernel,
	alsa-devel, devicetree, padma.kvr, kgene.kim, abrestic

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

On Wed, Aug 14, 2013 at 10:25:27AM +0200, Tomasz Figa wrote:

> Hmm, this series looks good to me too, except one thing. What about the 
> bisection breakage introduced by the hole between patches 1/4 and 4/4 
> (after the list of supported compatible changes in the driver and before 
> respective dts files are updated)?

It's annoying but relatively localised and obvious rather than the cross
tree stuff that happens so frequently with these platforms.

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

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

* [PATCH V4 0/4] Add i2s support on smdk5420
@ 2013-08-14 10:03       ` Mark Brown
  0 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2013-08-14 10:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Aug 14, 2013 at 10:25:27AM +0200, Tomasz Figa wrote:

> Hmm, this series looks good to me too, except one thing. What about the 
> bisection breakage introduced by the hole between patches 1/4 and 4/4 
> (after the list of supported compatible changes in the driver and before 
> respective dts files are updated)?

It's annoying but relatively localised and obvious rather than the cross
tree stuff that happens so frequently with these platforms.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130814/525e0a3c/attachment.sig>

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

end of thread, other threads:[~2013-08-14 10:03 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-12  9:49 [PATCH V4 0/4] Add i2s support on smdk5420 Padmavathi Venna
2013-08-12  9:49 ` Padmavathi Venna
2013-08-12  9:49 ` [PATCH V4 1/4] ASoC: Samsung: I2S: Add quirks as driver data in I2S Padmavathi Venna
2013-08-12  9:49   ` Padmavathi Venna
2013-08-12 12:06   ` Tomasz Figa
2013-08-12 12:06     ` Tomasz Figa
2013-08-12 22:57   ` Stephen Warren
2013-08-12 22:57     ` Stephen Warren
2013-08-12 23:13     ` Mark Brown
2013-08-12 23:13       ` Mark Brown
2013-08-12 23:18       ` Stephen Warren
2013-08-12 23:18         ` Stephen Warren
2013-08-12 23:46         ` Mark Brown
2013-08-12 23:46           ` Mark Brown
2013-08-13 15:42           ` Stephen Warren
2013-08-13 15:42             ` Stephen Warren
2013-08-12  9:49 ` [PATCH V4 2/4] ASoC: Samsung: I2S: Modify the I2S driver to support I2S on Exynos5420 Padmavathi Venna
2013-08-12  9:49   ` Padmavathi Venna
2013-08-12  9:49 ` [PATCH V4 3/4] ARM: dts: exynos5250: move common i2s properties to exynos5 dtsi Padmavathi Venna
2013-08-12  9:49   ` Padmavathi Venna
2013-08-12  9:49 ` [PATCH V4 4/4] ARM: dts: Change i2s compatible string on exynos5250 Padmavathi Venna
2013-08-12  9:49   ` Padmavathi Venna
2013-08-13 12:44 ` [PATCH V4 0/4] Add i2s support on smdk5420 Mark Brown
2013-08-13 12:44   ` Mark Brown
2013-08-14  8:25   ` Tomasz Figa
2013-08-14  8:25     ` Tomasz Figa
2013-08-14 10:03     ` Mark Brown
2013-08-14 10:03       ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.