[PATCH v3 0/3] ASoC: rockchip: i2s: add 8 channels capture support and share lrck

[PATCH v3 0/3] ASoC: rockchip: i2s: add 8 channels capture support and share lrck

[Sugar Zhang]

support 8 channels capture support.
share lrck tx/rx when symmetric_rates enabled.

Sugar Zhang (3):
  ASoC: rockchip: i2s: add 8 channels capture support
  Documentation: DT bindings: rockchip-i2s: add capture property
  ASoC: rockchip: i2s: share tx/rx lrck when symmetric_rates enabled

 .../devicetree/bindings/sound/rockchip-i2s.txt     |  5 +++
 sound/soc/rockchip/rockchip_i2s.c                  | 48 +++++++++++++++++++++-
 sound/soc/rockchip/rockchip_i2s.h                  | 16 ++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

-- 
2.3.6

[PATCH v3 0/3] ASoC: rockchip: i2s: add 8 channels capture support and share lrck

[Sugar Zhang]

support 8 channels capture support.
share lrck tx/rx when symmetric_rates enabled.

Sugar Zhang (3):
  ASoC: rockchip: i2s: add 8 channels capture support
  Documentation: DT bindings: rockchip-i2s: add capture property
  ASoC: rockchip: i2s: share tx/rx lrck when symmetric_rates enabled

 .../devicetree/bindings/sound/rockchip-i2s.txt     |  5 +++
 sound/soc/rockchip/rockchip_i2s.c                  | 48 +++++++++++++++++++++-
 sound/soc/rockchip/rockchip_i2s.h                  | 16 ++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

-- 
2.3.6

[PATCH v3 1/3] ASoC: rockchip: i2s: add 8 channels capture support

[Sugar Zhang]

support max 8 channels capture, please add property
'rockchip,capture-channels' in dts to enable this,
if not, support 2 channels capture default.

Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
---

Changes in v3: None

Changes in v2:
- remove lrck mode, and move it to new patch

 sound/soc/rockchip/rockchip_i2s.c | 40 +++++++++++++++++++++++++++++++++++++--
 sound/soc/rockchip/rockchip_i2s.h | 10 ++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index b936102..f07833b 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -245,8 +245,34 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	regmap_update_bits(i2s->regmap, I2S_TXCR, I2S_TXCR_VDW_MASK, val);
-	regmap_update_bits(i2s->regmap, I2S_RXCR, I2S_RXCR_VDW_MASK, val);
+	switch (params_channels(params)) {
+	case 8:
+		val |= I2S_CHN_8;
+		break;
+	case 6:
+		val |= I2S_CHN_6;
+		break;
+	case 4:
+		val |= I2S_CHN_4;
+		break;
+	case 2:
+		val |= I2S_CHN_2;
+		break;
+	default:
+		dev_err(i2s->dev, "invalid channel: %d\n",
+			params_channels(params));
+		return -EINVAL;
+	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		regmap_update_bits(i2s->regmap, I2S_RXCR,
+				   I2S_RXCR_VDW_MASK | I2S_RXCR_CSR_MASK,
+				   val);
+	else
+		regmap_update_bits(i2s->regmap, I2S_TXCR,
+				   I2S_TXCR_VDW_MASK | I2S_TXCR_CSR_MASK,
+				   val);
+
 	regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_TDL_MASK,
 			   I2S_DMACR_TDL(16));
 	regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK,
@@ -415,10 +441,12 @@ static const struct regmap_config rockchip_i2s_regmap_config = {
 
 static int rockchip_i2s_probe(struct platform_device *pdev)
 {
+	struct device_node *node = pdev->dev.of_node;
 	struct rk_i2s_dev *i2s;
 	struct resource *res;
 	void __iomem *regs;
 	int ret;
+	int val;
 
 	i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
 	if (!i2s) {
@@ -475,6 +503,14 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
 			goto err_pm_disable;
 	}
 
+	/* refine capture channels */
+	if (!of_property_read_u32(node, "rockchip,capture-channels", &val)) {
+		if (val >= 2 && val <= 8)
+			rockchip_i2s_dai.capture.channels_max = val;
+		else
+			rockchip_i2s_dai.capture.channels_max = 2;
+	}
+
 	ret = devm_snd_soc_register_component(&pdev->dev,
 					      &rockchip_i2s_component,
 					      &rockchip_i2s_dai, 1);
diff --git a/sound/soc/rockchip/rockchip_i2s.h b/sound/soc/rockchip/rockchip_i2s.h
index 93f456f..a54ee35 100644
--- a/sound/soc/rockchip/rockchip_i2s.h
+++ b/sound/soc/rockchip/rockchip_i2s.h
@@ -49,6 +49,9 @@
  * RXCR
  * receive operation control register
 */
+#define I2S_RXCR_CSR_SHIFT	15
+#define I2S_RXCR_CSR(x)		(x << I2S_RXCR_CSR_SHIFT)
+#define I2S_RXCR_CSR_MASK	(3 << I2S_RXCR_CSR_SHIFT)
 #define I2S_RXCR_HWT		BIT(14)
 #define I2S_RXCR_SJM_SHIFT	12
 #define I2S_RXCR_SJM_R		(0 << I2S_RXCR_SJM_SHIFT)
@@ -207,6 +210,13 @@ enum {
 	ROCKCHIP_DIV_BCLK,
 };
 
+/* channel select */
+#define I2S_CSR_SHIFT	15
+#define I2S_CHN_2	(0 << I2S_CSR_SHIFT)
+#define I2S_CHN_4	(1 << I2S_CSR_SHIFT)
+#define I2S_CHN_6	(2 << I2S_CSR_SHIFT)
+#define I2S_CHN_8	(3 << I2S_CSR_SHIFT)
+
 /* I2S REGS */
 #define I2S_TXCR	(0x0000)
 #define I2S_RXCR	(0x0004)
-- 
2.3.6

[PATCH v3 1/3] ASoC: rockchip: i2s: add 8 channels capture support

[Sugar Zhang]

support max 8 channels capture, please add property
'rockchip,capture-channels' in dts to enable this,
if not, support 2 channels capture default.

Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
---

Changes in v3: None

Changes in v2:
- remove lrck mode, and move it to new patch

 sound/soc/rockchip/rockchip_i2s.c | 40 +++++++++++++++++++++++++++++++++++++--
 sound/soc/rockchip/rockchip_i2s.h | 10 ++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index b936102..f07833b 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -245,8 +245,34 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	regmap_update_bits(i2s->regmap, I2S_TXCR, I2S_TXCR_VDW_MASK, val);
-	regmap_update_bits(i2s->regmap, I2S_RXCR, I2S_RXCR_VDW_MASK, val);
+	switch (params_channels(params)) {
+	case 8:
+		val |= I2S_CHN_8;
+		break;
+	case 6:
+		val |= I2S_CHN_6;
+		break;
+	case 4:
+		val |= I2S_CHN_4;
+		break;
+	case 2:
+		val |= I2S_CHN_2;
+		break;
+	default:
+		dev_err(i2s->dev, "invalid channel: %d\n",
+			params_channels(params));
+		return -EINVAL;
+	}
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		regmap_update_bits(i2s->regmap, I2S_RXCR,
+				   I2S_RXCR_VDW_MASK | I2S_RXCR_CSR_MASK,
+				   val);
+	else
+		regmap_update_bits(i2s->regmap, I2S_TXCR,
+				   I2S_TXCR_VDW_MASK | I2S_TXCR_CSR_MASK,
+				   val);
+
 	regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_TDL_MASK,
 			   I2S_DMACR_TDL(16));
 	regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK,
@@ -415,10 +441,12 @@ static const struct regmap_config rockchip_i2s_regmap_config = {
 
 static int rockchip_i2s_probe(struct platform_device *pdev)
 {
+	struct device_node *node = pdev->dev.of_node;
 	struct rk_i2s_dev *i2s;
 	struct resource *res;
 	void __iomem *regs;
 	int ret;
+	int val;
 
 	i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
 	if (!i2s) {
@@ -475,6 +503,14 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
 			goto err_pm_disable;
 	}
 
+	/* refine capture channels */
+	if (!of_property_read_u32(node, "rockchip,capture-channels", &val)) {
+		if (val >= 2 && val <= 8)
+			rockchip_i2s_dai.capture.channels_max = val;
+		else
+			rockchip_i2s_dai.capture.channels_max = 2;
+	}
+
 	ret = devm_snd_soc_register_component(&pdev->dev,
 					      &rockchip_i2s_component,
 					      &rockchip_i2s_dai, 1);
diff --git a/sound/soc/rockchip/rockchip_i2s.h b/sound/soc/rockchip/rockchip_i2s.h
index 93f456f..a54ee35 100644
--- a/sound/soc/rockchip/rockchip_i2s.h
+++ b/sound/soc/rockchip/rockchip_i2s.h
@@ -49,6 +49,9 @@
  * RXCR
  * receive operation control register
 */
+#define I2S_RXCR_CSR_SHIFT	15
+#define I2S_RXCR_CSR(x)		(x << I2S_RXCR_CSR_SHIFT)
+#define I2S_RXCR_CSR_MASK	(3 << I2S_RXCR_CSR_SHIFT)
 #define I2S_RXCR_HWT		BIT(14)
 #define I2S_RXCR_SJM_SHIFT	12
 #define I2S_RXCR_SJM_R		(0 << I2S_RXCR_SJM_SHIFT)
@@ -207,6 +210,13 @@ enum {
 	ROCKCHIP_DIV_BCLK,
 };
 
+/* channel select */
+#define I2S_CSR_SHIFT	15
+#define I2S_CHN_2	(0 << I2S_CSR_SHIFT)
+#define I2S_CHN_4	(1 << I2S_CSR_SHIFT)
+#define I2S_CHN_6	(2 << I2S_CSR_SHIFT)
+#define I2S_CHN_8	(3 << I2S_CSR_SHIFT)
+
 /* I2S REGS */
 #define I2S_TXCR	(0x0000)
 #define I2S_RXCR	(0x0004)
-- 
2.3.6

[PATCH v3 2/3] Documentation: DT bindings: rockchip-i2s: add capture property

[Sugar Zhang]

rockchip,capture-channels: max capture channels, 2 channels default.

Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
---

Changes in v3:
- move property into the Optional properties section

Changes in v2:
- remove unused lrck property

 Documentation/devicetree/bindings/sound/rockchip-i2s.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
index 9b82c20..ace3b1e 100644
--- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
@@ -22,6 +22,10 @@ Required properties:
    - "i2s_hclk": clock for I2S BUS
    - "i2s_clk" : clock for I2S controller
 
+Optional properties:
+
+- rockchip,capture-channels: max capture channels, if not set, 2 channels default.
+
 Example for rk3288 I2S controller:
 
 i2s@ff890000 {
@@ -34,4 +38,5 @@ i2s@ff890000 {
 	dma-names = "tx", "rx";
 	clock-names = "i2s_hclk", "i2s_clk";
 	clocks = <&cru HCLK_I2S0>, <&cru SCLK_I2S0>;
+	rockchip,capture-channels = <2>;
 };
-- 
2.3.6

[PATCH v3 2/3] Documentation: DT bindings: rockchip-i2s: add capture property

[Sugar Zhang]

rockchip,capture-channels: max capture channels, 2 channels default.

Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
---

Changes in v3:
- move property into the Optional properties section

Changes in v2:
- remove unused lrck property

 Documentation/devicetree/bindings/sound/rockchip-i2s.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
index 9b82c20..ace3b1e 100644
--- a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
@@ -22,6 +22,10 @@ Required properties:
    - "i2s_hclk": clock for I2S BUS
    - "i2s_clk" : clock for I2S controller
 
+Optional properties:
+
+- rockchip,capture-channels: max capture channels, if not set, 2 channels default.
+
 Example for rk3288 I2S controller:
 
 i2s at ff890000 {
@@ -34,4 +38,5 @@ i2s at ff890000 {
 	dma-names = "tx", "rx";
 	clock-names = "i2s_hclk", "i2s_clk";
 	clocks = <&cru HCLK_I2S0>, <&cru SCLK_I2S0>;
+	rockchip,capture-channels = <2>;
 };
-- 
2.3.6

[PATCH v3 3/3] ASoC: rockchip: i2s: share tx/rx lrck when symmetric_rates enabled

[Sugar Zhang]

we share lrck_tx to lrck_rx only if when both i2s symmetric_rates and
dai_link symmetric_rates enabled.

if only lrck_tx is wired into external codec, we should enable this to
provider lrck for rx logic inside i2s module.

Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
---

Changes in v3:
- modify the judgment condition

Changes in v2:
- new patch

 sound/soc/rockchip/rockchip_i2s.c | 8 ++++++++
 sound/soc/rockchip/rockchip_i2s.h | 6 ++++++
 2 files changed, 14 insertions(+)

diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index f07833b..5190199 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -226,6 +226,7 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
 				  struct snd_soc_dai *dai)
 {
 	struct rk_i2s_dev *i2s = to_info(dai);
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	unsigned int val = 0;
 
 	switch (params_format(params)) {
@@ -278,6 +279,13 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
 	regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK,
 			   I2S_DMACR_RDL(16));
 
+	val = I2S_CKR_TRCM_TXRX;
+	if (dai->driver->symmetric_rates && rtd->dai_link->symmetric_rates)
+		val = I2S_CKR_TRCM_TXSHARE;
+
+	regmap_update_bits(i2s->regmap, I2S_CKR,
+			   I2S_CKR_TRCM_MASK,
+			   val);
 	return 0;
 }
 
diff --git a/sound/soc/rockchip/rockchip_i2s.h b/sound/soc/rockchip/rockchip_i2s.h
index a54ee35..dc6e2c7 100644
--- a/sound/soc/rockchip/rockchip_i2s.h
+++ b/sound/soc/rockchip/rockchip_i2s.h
@@ -78,6 +78,12 @@
  * CKR
  * clock generation register
 */
+#define I2S_CKR_TRCM_SHIFT	28
+#define I2S_CKR_TRCM(x)	(x << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_TXRX	(0 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_TXSHARE	(1 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_RXSHARE	(2 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_MASK	(3 << I2S_CKR_TRCM_SHIFT)
 #define I2S_CKR_MSS_SHIFT	27
 #define I2S_CKR_MSS_MASTER	(0 << I2S_CKR_MSS_SHIFT)
 #define I2S_CKR_MSS_SLAVE	(1 << I2S_CKR_MSS_SHIFT)
-- 
2.3.6

[PATCH v3 3/3] ASoC: rockchip: i2s: share tx/rx lrck when symmetric_rates enabled

[Sugar Zhang]

we share lrck_tx to lrck_rx only if when both i2s symmetric_rates and
dai_link symmetric_rates enabled.

if only lrck_tx is wired into external codec, we should enable this to
provider lrck for rx logic inside i2s module.

Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com>
---

Changes in v3:
- modify the judgment condition

Changes in v2:
- new patch

 sound/soc/rockchip/rockchip_i2s.c | 8 ++++++++
 sound/soc/rockchip/rockchip_i2s.h | 6 ++++++
 2 files changed, 14 insertions(+)

diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index f07833b..5190199 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -226,6 +226,7 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
 				  struct snd_soc_dai *dai)
 {
 	struct rk_i2s_dev *i2s = to_info(dai);
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	unsigned int val = 0;
 
 	switch (params_format(params)) {
@@ -278,6 +279,13 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
 	regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK,
 			   I2S_DMACR_RDL(16));
 
+	val = I2S_CKR_TRCM_TXRX;
+	if (dai->driver->symmetric_rates && rtd->dai_link->symmetric_rates)
+		val = I2S_CKR_TRCM_TXSHARE;
+
+	regmap_update_bits(i2s->regmap, I2S_CKR,
+			   I2S_CKR_TRCM_MASK,
+			   val);
 	return 0;
 }
 
diff --git a/sound/soc/rockchip/rockchip_i2s.h b/sound/soc/rockchip/rockchip_i2s.h
index a54ee35..dc6e2c7 100644
--- a/sound/soc/rockchip/rockchip_i2s.h
+++ b/sound/soc/rockchip/rockchip_i2s.h
@@ -78,6 +78,12 @@
  * CKR
  * clock generation register
 */
+#define I2S_CKR_TRCM_SHIFT	28
+#define I2S_CKR_TRCM(x)	(x << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_TXRX	(0 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_TXSHARE	(1 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_RXSHARE	(2 << I2S_CKR_TRCM_SHIFT)
+#define I2S_CKR_TRCM_MASK	(3 << I2S_CKR_TRCM_SHIFT)
 #define I2S_CKR_MSS_SHIFT	27
 #define I2S_CKR_MSS_MASTER	(0 << I2S_CKR_MSS_SHIFT)
 #define I2S_CKR_MSS_SLAVE	(1 << I2S_CKR_MSS_SHIFT)
-- 
2.3.6

Re: [PATCH v3 1/3] ASoC: rockchip: i2s: add 8 channels capture support

[Mark Brown]

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

On Fri, Nov 06, 2015 at 09:38:09AM +0800, Sugar Zhang wrote:

> +	/* refine capture channels */
> +	if (!of_property_read_u32(node, "rockchip,capture-channels", &val)) {
> +		if (val >= 2 && val <= 8)
> +			rockchip_i2s_dai.capture.channels_max = val;
> +		else
> +			rockchip_i2s_dai.capture.channels_max = 2;
> +	}

You should make a copy of the rockchip_i2s_dai struct in your driver
data and modify that rather than modifying the original - what happens
if sommeone makes a SoC with two I2S DAIs, one with many channels and
another with only two?  Samsung SoCs often have an arrangement like that
for exmaple.

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

[PATCH v3 1/3] ASoC: rockchip: i2s: add 8 channels capture support

[Mark Brown]

On Fri, Nov 06, 2015 at 09:38:09AM +0800, Sugar Zhang wrote:

> +	/* refine capture channels */
> +	if (!of_property_read_u32(node, "rockchip,capture-channels", &val)) {
> +		if (val >= 2 && val <= 8)
> +			rockchip_i2s_dai.capture.channels_max = val;
> +		else
> +			rockchip_i2s_dai.capture.channels_max = 2;
> +	}

You should make a copy of the rockchip_i2s_dai struct in your driver
data and modify that rather than modifying the original - what happens
if sommeone makes a SoC with two I2S DAIs, one with many channels and
another with only two?  Samsung SoCs often have an arrangement like that
for exmaple.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151106/abfa0950/attachment-0001.sig>

Re: [PATCH v3 3/3] ASoC: rockchip: i2s: share tx/rx lrck when symmetric_rates enabled

[Mark Brown]

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

On Fri, Nov 06, 2015 at 09:38:11AM +0800, Sugar Zhang wrote:
> we share lrck_tx to lrck_rx only if when both i2s symmetric_rates and
> dai_link symmetric_rates enabled.

This looks good but seems to depend on patch 1 so I can't apply it yet.

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

[PATCH v3 3/3] ASoC: rockchip: i2s: share tx/rx lrck when symmetric_rates enabled

[Mark Brown]

On Fri, Nov 06, 2015 at 09:38:11AM +0800, Sugar Zhang wrote:
> we share lrck_tx to lrck_rx only if when both i2s symmetric_rates and
> dai_link symmetric_rates enabled.

This looks good but seems to depend on patch 1 so I can't apply it yet.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151106/9d340c69/attachment.sig>

Re: [PATCH v3 1/3] ASoC: rockchip: i2s: add 8 channels capture support

[sugar]

On 11/6/2015 20:00, Mark Brown wrote:
> On Fri, Nov 06, 2015 at 09:38:09AM +0800, Sugar Zhang wrote:
>
>> +	/* refine capture channels */
>> +	if (!of_property_read_u32(node, "rockchip,capture-channels", &val)) {
>> +		if (val >= 2 && val <= 8)
>> +			rockchip_i2s_dai.capture.channels_max = val;
>> +		else
>> +			rockchip_i2s_dai.capture.channels_max = 2;
>> +	}
>
> You should make a copy of the rockchip_i2s_dai struct in your driver
> data and modify that rather than modifying the original - what happens
> if sommeone makes a SoC with two I2S DAIs, one with many channels and
> another with only two?  Samsung SoCs often have an arrangement like that
> for exmaple.
>

Yes, I agree, because this patch was merged into mainline, I will post a 
new one based on the latest code to fix this, thanks.
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>

Re: [PATCH v3 1/3] ASoC: rockchip: i2s: add 8 channels capture support

[sugar]

On 11/6/2015 20:00, Mark Brown wrote:
> On Fri, Nov 06, 2015 at 09:38:09AM +0800, Sugar Zhang wrote:
>
>> +	/* refine capture channels */
>> +	if (!of_property_read_u32(node, "rockchip,capture-channels", &val)) {
>> +		if (val >= 2 && val <= 8)
>> +			rockchip_i2s_dai.capture.channels_max = val;
>> +		else
>> +			rockchip_i2s_dai.capture.channels_max = 2;
>> +	}
>
> You should make a copy of the rockchip_i2s_dai struct in your driver
> data and modify that rather than modifying the original - what happens
> if sommeone makes a SoC with two I2S DAIs, one with many channels and
> another with only two?  Samsung SoCs often have an arrangement like that
> for exmaple.
>

Yes, I agree, because this patch was merged into mainline, I will post a 
new one based on the latest code to fix this, thanks.
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>

[PATCH v3 1/3] ASoC: rockchip: i2s: add 8 channels capture support

[sugar]

On 11/6/2015 20:00, Mark Brown wrote:
> On Fri, Nov 06, 2015 at 09:38:09AM +0800, Sugar Zhang wrote:
>
>> +	/* refine capture channels */
>> +	if (!of_property_read_u32(node, "rockchip,capture-channels", &val)) {
>> +		if (val >= 2 && val <= 8)
>> +			rockchip_i2s_dai.capture.channels_max = val;
>> +		else
>> +			rockchip_i2s_dai.capture.channels_max = 2;
>> +	}
>
> You should make a copy of the rockchip_i2s_dai struct in your driver
> data and modify that rather than modifying the original - what happens
> if sommeone makes a SoC with two I2S DAIs, one with many channels and
> another with only two?  Samsung SoCs often have an arrangement like that
> for exmaple.
>

Yes, I agree, because this patch was merged into mainline, I will post a 
new one based on the latest code to fix this, thanks.
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>