linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add dual-fifo mode support of i.MX ssi
@ 2013-10-29 12:33 Nicolin Chen
  2013-10-29 12:33 ` [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support Nicolin Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Nicolin Chen @ 2013-10-29 12:33 UTC (permalink / raw)
  To: timur, shawn.guo, broonie
  Cc: linux-arm-kernel, devicetree, linux-doc, linux-kernel, dmaengine,
	linuxppc-dev, alsa-devel, rob.herring, mark.rutland, swarren,
	pawel.moll, ijc+devicetree, s.hauer, b32955

Changelog
v1:
 * SSI can reduce hardware overrun/underrun possibility when using dual
 * fifo mode. To support this mode, we need to first update sdma sciprt
 * list, and then enable dual fifo BIT in SSI driver, and last update DT
 * bindings of i.MX series.
 *
 * ! This series of patches has a direct dependency between them. When
 * ! applying them, we need to apply in one single branch. Otherwise,
 * ! it would break currect branches.

Nicolin Chen (3):
  dma: imx-sdma: Add ssi dual fifo script support
  ASoC: fsl_ssi: Add dual fifo mode support
  ARM: dts: imx: use dual-fifo sdma script for ssi

 .../devicetree/bindings/dma/fsl-imx-sdma.txt       |  1 +
 arch/arm/boot/dts/imx51.dtsi                       |  4 ++--
 arch/arm/boot/dts/imx53.dtsi                       |  4 ++--
 arch/arm/boot/dts/imx6qdl.dtsi                     | 12 +++++------
 arch/arm/boot/dts/imx6sl.dtsi                      | 12 +++++------
 drivers/dma/imx-sdma.c                             |  6 +++++-
 include/linux/platform_data/dma-imx-sdma.h         |  2 ++
 include/linux/platform_data/dma-imx.h              |  1 +
 sound/soc/fsl/fsl_ssi.c                            | 24 +++++++++++++++++++++-
 9 files changed, 48 insertions(+), 18 deletions(-)

-- 
1.8.4



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

* [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support
  2013-10-29 12:33 [PATCH 0/3] Add dual-fifo mode support of i.MX ssi Nicolin Chen
@ 2013-10-29 12:33 ` Nicolin Chen
  2013-10-29 13:51   ` Sascha Hauer
  2013-10-29 17:21   ` Kumar Gala
  2013-10-29 12:33 ` [PATCH 2/3] ASoC: fsl_ssi: Add dual fifo mode support Nicolin Chen
  2013-10-29 12:33 ` [PATCH 3/3] ARM: dts: imx: use dual-fifo sdma script for ssi Nicolin Chen
  2 siblings, 2 replies; 11+ messages in thread
From: Nicolin Chen @ 2013-10-29 12:33 UTC (permalink / raw)
  To: timur, shawn.guo, broonie
  Cc: linux-arm-kernel, devicetree, linux-doc, linux-kernel, dmaengine,
	linuxppc-dev, alsa-devel, rob.herring, mark.rutland, swarren,
	pawel.moll, ijc+devicetree, s.hauer, b32955

There's a script for SSI missing in current sdma script list. Thus add it.
This script would allow SSI use its dual fifo mode to transimit/receive
data without occasional hardware underrun/overrun.

This patch also fixed a counting error for total number of scripts.

Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
 Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
 drivers/dma/imx-sdma.c                                 | 6 +++++-
 include/linux/platform_data/dma-imx-sdma.h             | 2 ++
 include/linux/platform_data/dma-imx.h                  | 1 +
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
index 4fa814d..3b933c5 100644
--- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
@@ -42,6 +42,7 @@ The full ID of peripheral types can be found below.
 	19	IPU Memory
 	20	ASRC
 	21	ESAI
+	22	SSI Dual FIFO
 
 The third cell specifies the transfer priority as below.
 
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index fc43603..695871f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -724,6 +724,10 @@ static void sdma_get_pc(struct sdma_channel *sdmac,
 		per_2_emi = sdma->script_addrs->app_2_mcu_addr;
 		emi_2_per = sdma->script_addrs->mcu_2_app_addr;
 		break;
+	case IMX_DMATYPE_SSI_DUAL:
+		per_2_emi = sdma->script_addrs->ssish_2_mcu_addr;
+		emi_2_per = sdma->script_addrs->mcu_2_ssish_addr;
+		break;
 	case IMX_DMATYPE_SSI_SP:
 	case IMX_DMATYPE_MMC:
 	case IMX_DMATYPE_SDHC:
@@ -1237,7 +1241,7 @@ static void sdma_issue_pending(struct dma_chan *chan)
 		sdma_enable_channel(sdma, sdmac->channel);
 }
 
-#define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1	34
+#define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1	37
 
 static void sdma_add_scripts(struct sdma_engine *sdma,
 		const struct sdma_script_start_addrs *addr)
diff --git a/include/linux/platform_data/dma-imx-sdma.h b/include/linux/platform_data/dma-imx-sdma.h
index 3a39428..19cfa9a 100644
--- a/include/linux/platform_data/dma-imx-sdma.h
+++ b/include/linux/platform_data/dma-imx-sdma.h
@@ -43,6 +43,8 @@ struct sdma_script_start_addrs {
 	s32 dptc_dvfs_addr;
 	s32 utra_addr;
 	s32 ram_code_start_addr;
+	s32 mcu_2_ssish_addr;
+	s32 ssish_2_mcu_addr;
 };
 
 /**
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
index beac6b8..bcbc6c3 100644
--- a/include/linux/platform_data/dma-imx.h
+++ b/include/linux/platform_data/dma-imx.h
@@ -39,6 +39,7 @@ enum sdma_peripheral_type {
 	IMX_DMATYPE_IPU_MEMORY,	/* IPU Memory */
 	IMX_DMATYPE_ASRC,	/* ASRC */
 	IMX_DMATYPE_ESAI,	/* ESAI */
+	IMX_DMATYPE_SSI_DUAL,	/* SSI Dual FIFO */
 };
 
 enum imx_dma_prio {
-- 
1.8.4



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

* [PATCH 2/3] ASoC: fsl_ssi: Add dual fifo mode support
  2013-10-29 12:33 [PATCH 0/3] Add dual-fifo mode support of i.MX ssi Nicolin Chen
  2013-10-29 12:33 ` [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support Nicolin Chen
@ 2013-10-29 12:33 ` Nicolin Chen
  2013-10-29 12:42   ` Timur Tabi
  2013-10-29 12:33 ` [PATCH 3/3] ARM: dts: imx: use dual-fifo sdma script for ssi Nicolin Chen
  2 siblings, 1 reply; 11+ messages in thread
From: Nicolin Chen @ 2013-10-29 12:33 UTC (permalink / raw)
  To: timur, shawn.guo, broonie
  Cc: linux-arm-kernel, devicetree, linux-doc, linux-kernel, dmaengine,
	linuxppc-dev, alsa-devel, rob.herring, mark.rutland, swarren,
	pawel.moll, ijc+devicetree, s.hauer, b32955

By enabling dual fifo mode, it would allow SSI enter a better performance
to transimit/receive data without occasional hardware underrun/overrun.

[ Passed compile-test with mpc85xx_defconfig ]

Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
 sound/soc/fsl/fsl_ssi.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 3797bf0..7ad01ce 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -146,6 +146,7 @@ struct fsl_ssi_private {
 	bool ssi_on_imx;
 	bool imx_ac97;
 	bool use_dma;
+	bool use_dual_fifo;
 	struct clk *clk;
 	struct snd_dmaengine_dai_dma_data dma_params_tx;
 	struct snd_dmaengine_dai_dma_data dma_params_rx;
@@ -416,6 +417,16 @@ static int fsl_ssi_setup(struct fsl_ssi_private *ssi_private)
 		write_ssi(CCSR_SSI_SOR_WAIT(3), &ssi->sor);
 	}
 
+	if (ssi_private->use_dual_fifo) {
+		write_ssi_mask(&ssi->srcr, 0, CCSR_SSI_SRCR_RFEN1);
+		write_ssi_mask(&ssi->stcr, 0, CCSR_SSI_STCR_TFEN1);
+		write_ssi_mask(&ssi->scr, 0, CCSR_SSI_SCR_TCH_EN);
+	} else {
+		write_ssi_mask(&ssi->srcr, CCSR_SSI_SRCR_RFEN1, 0);
+		write_ssi_mask(&ssi->stcr, CCSR_SSI_STCR_TFEN1, 0);
+		write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_TCH_EN, 0);
+	}
+
 	return 0;
 }
 
@@ -952,7 +963,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 		ssi_private->fifo_depth = 8;
 
 	if (of_device_is_compatible(pdev->dev.of_node, "fsl,imx21-ssi")) {
-		u32 dma_events[2];
+		u32 dma_events[2], dmas[4];
 		ssi_private->ssi_on_imx = true;
 
 		ssi_private->clk = devm_clk_get(&pdev->dev, NULL);
@@ -1006,6 +1017,17 @@ static int fsl_ssi_probe(struct platform_device *pdev)
 			dma_events[0], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI);
 		imx_pcm_dma_params_init_data(&ssi_private->filter_data_rx,
 			dma_events[1], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI);
+		if (!of_property_read_u32_array(pdev->dev.of_node, "dmas", dmas, 4)
+				&& dmas[2] == IMX_DMATYPE_SSI_DUAL) {
+			ssi_private->use_dual_fifo = true;
+			/* When using dual fifo mode, we need to keep watermark
+			 * as even numbers due to dma script limitation.
+			 */
+			ssi_private->dma_params_tx.maxburst /= 2;
+			ssi_private->dma_params_tx.maxburst *= 2;
+			ssi_private->dma_params_rx.maxburst /= 2;
+			ssi_private->dma_params_rx.maxburst *= 2;
+		}
 	} else if (ssi_private->use_dma) {
 		/* The 'name' should not have any slashes in it. */
 		ret = devm_request_irq(&pdev->dev, ssi_private->irq,
-- 
1.8.4



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

* [PATCH 3/3] ARM: dts: imx: use dual-fifo sdma script for ssi
  2013-10-29 12:33 [PATCH 0/3] Add dual-fifo mode support of i.MX ssi Nicolin Chen
  2013-10-29 12:33 ` [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support Nicolin Chen
  2013-10-29 12:33 ` [PATCH 2/3] ASoC: fsl_ssi: Add dual fifo mode support Nicolin Chen
@ 2013-10-29 12:33 ` Nicolin Chen
  2 siblings, 0 replies; 11+ messages in thread
From: Nicolin Chen @ 2013-10-29 12:33 UTC (permalink / raw)
  To: timur, shawn.guo, broonie
  Cc: linux-arm-kernel, devicetree, linux-doc, linux-kernel, dmaengine,
	linuxppc-dev, alsa-devel, rob.herring, mark.rutland, swarren,
	pawel.moll, ijc+devicetree, s.hauer, b32955

Use dual-fifo sdma scripts instead of shared scripts for ssi on i.MX series.

Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
 arch/arm/boot/dts/imx51.dtsi   |  4 ++--
 arch/arm/boot/dts/imx53.dtsi   |  4 ++--
 arch/arm/boot/dts/imx6qdl.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx6sl.dtsi  | 12 ++++++------
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 54cee65..1a71eac 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -154,8 +154,8 @@
 					reg = <0x70014000 0x4000>;
 					interrupts = <30>;
 					clocks = <&clks 49>;
-					dmas = <&sdma 24 1 0>,
-					       <&sdma 25 1 0>;
+					dmas = <&sdma 24 22 0>,
+					       <&sdma 25 22 0>;
 					dma-names = "rx", "tx";
 					fsl,fifo-depth = <15>;
 					fsl,ssi-dma-events = <25 24 23 22>; /* TX0 RX0 TX1 RX1 */
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 4307e80..7208fde 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -153,8 +153,8 @@
 					reg = <0x50014000 0x4000>;
 					interrupts = <30>;
 					clocks = <&clks 49>;
-					dmas = <&sdma 24 1 0>,
-					       <&sdma 25 1 0>;
+					dmas = <&sdma 24 22 0>,
+					       <&sdma 25 22 0>;
 					dma-names = "rx", "tx";
 					fsl,fifo-depth = <15>;
 					fsl,ssi-dma-events = <25 24 23 22>; /* TX0 RX0 TX1 RX1 */
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 57e9c38..6e096ca 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -223,8 +223,8 @@
 					reg = <0x02028000 0x4000>;
 					interrupts = <0 46 0x04>;
 					clocks = <&clks 178>;
-					dmas = <&sdma 37 1 0>,
-					       <&sdma 38 1 0>;
+					dmas = <&sdma 37 22 0>,
+					       <&sdma 38 22 0>;
 					dma-names = "rx", "tx";
 					fsl,fifo-depth = <15>;
 					fsl,ssi-dma-events = <38 37>;
@@ -236,8 +236,8 @@
 					reg = <0x0202c000 0x4000>;
 					interrupts = <0 47 0x04>;
 					clocks = <&clks 179>;
-					dmas = <&sdma 41 1 0>,
-					       <&sdma 42 1 0>;
+					dmas = <&sdma 41 22 0>,
+					       <&sdma 42 22 0>;
 					dma-names = "rx", "tx";
 					fsl,fifo-depth = <15>;
 					fsl,ssi-dma-events = <42 41>;
@@ -249,8 +249,8 @@
 					reg = <0x02030000 0x4000>;
 					interrupts = <0 48 0x04>;
 					clocks = <&clks 180>;
-					dmas = <&sdma 45 1 0>,
-					       <&sdma 46 1 0>;
+					dmas = <&sdma 45 22 0>,
+					       <&sdma 46 22 0>;
 					dma-names = "rx", "tx";
 					fsl,fifo-depth = <15>;
 					fsl,ssi-dma-events = <46 45>;
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index c46651e..b32ba99 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -195,8 +195,8 @@
 					reg = <0x02028000 0x4000>;
 					interrupts = <0 46 0x04>;
 					clocks = <&clks IMX6SL_CLK_SSI1>;
-					dmas = <&sdma 37 1 0>,
-					       <&sdma 38 1 0>;
+					dmas = <&sdma 37 22 0>,
+					       <&sdma 38 22 0>;
 					dma-names = "rx", "tx";
 					fsl,fifo-depth = <15>;
 					status = "disabled";
@@ -207,8 +207,8 @@
 					reg = <0x0202c000 0x4000>;
 					interrupts = <0 47 0x04>;
 					clocks = <&clks IMX6SL_CLK_SSI2>;
-					dmas = <&sdma 41 1 0>,
-					       <&sdma 42 1 0>;
+					dmas = <&sdma 41 22 0>,
+					       <&sdma 42 22 0>;
 					dma-names = "rx", "tx";
 					fsl,fifo-depth = <15>;
 					status = "disabled";
@@ -219,8 +219,8 @@
 					reg = <0x02030000 0x4000>;
 					interrupts = <0 48 0x04>;
 					clocks = <&clks IMX6SL_CLK_SSI3>;
-					dmas = <&sdma 45 1 0>,
-					       <&sdma 46 1 0>;
+					dmas = <&sdma 45 22 0>,
+					       <&sdma 46 22 0>;
 					dma-names = "rx", "tx";
 					fsl,fifo-depth = <15>;
 					status = "disabled";
-- 
1.8.4



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

* Re: [PATCH 2/3] ASoC: fsl_ssi: Add dual fifo mode support
  2013-10-29 12:33 ` [PATCH 2/3] ASoC: fsl_ssi: Add dual fifo mode support Nicolin Chen
@ 2013-10-29 12:42   ` Timur Tabi
  2013-10-29 12:53     ` Chen Guangyu-B42378
  0 siblings, 1 reply; 11+ messages in thread
From: Timur Tabi @ 2013-10-29 12:42 UTC (permalink / raw)
  To: Nicolin Chen, shawn.guo, broonie
  Cc: linux-arm-kernel, devicetree, linux-doc, linux-kernel, dmaengine,
	linuxppc-dev, alsa-devel, rob.herring, mark.rutland, swarren,
	pawel.moll, ijc+devicetree, s.hauer, b32955

Nicolin Chen wrote:
> By enabling dual fifo mode, it would allow SSI enter a better performance
> to transimit/receive data without occasional hardware underrun/overrun.

Have you measured any real performance gain with this patch?  I 
considered adding dual-FIFO support when I originally wrote this driver, 
but it didn't appear to have any real benefit, but it used twice as many 
DMA channels.

I'm concerned that this is another patch that just enables a useless 
feature.

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

* Re: [PATCH 2/3] ASoC: fsl_ssi: Add dual fifo mode support
  2013-10-29 12:42   ` Timur Tabi
@ 2013-10-29 12:53     ` Chen Guangyu-B42378
  2013-10-29 13:00       ` Timur Tabi
  0 siblings, 1 reply; 11+ messages in thread
From: Chen Guangyu-B42378 @ 2013-10-29 12:53 UTC (permalink / raw)
  To: Timur Tabi
  Cc: shawn.guo, broonie, linux-arm-kernel, devicetree, linux-doc,
	linux-kernel, dmaengine, linuxppc-dev, alsa-devel, rob.herring,
	mark.rutland, swarren, pawel.moll, ijc+devicetree, s.hauer,
	Huang Shijie-B32955

Without dual fifo support,  handware underrun would occasionally occur and then two audio channels would physically swap. This could be easily reproduced in low bus frequency situation, while it would be better if we enable dual fifo.

Sent by Android device.

Timur Tabi <timur@tabi.org> wrote:


Nicolin Chen wrote:
> By enabling dual fifo mode, it would allow SSI enter a better performance
> to transimit/receive data without occasional hardware underrun/overrun.

Have you measured any real performance gain with this patch?  I
considered adding dual-FIFO support when I originally wrote this driver,
but it didn't appear to have any real benefit, but it used twice as many
DMA channels.

I'm concerned that this is another patch that just enables a useless
feature.



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

* Re: [PATCH 2/3] ASoC: fsl_ssi: Add dual fifo mode support
  2013-10-29 12:53     ` Chen Guangyu-B42378
@ 2013-10-29 13:00       ` Timur Tabi
  2013-10-29 13:03         ` Chen Guangyu-B42378
  0 siblings, 1 reply; 11+ messages in thread
From: Timur Tabi @ 2013-10-29 13:00 UTC (permalink / raw)
  To: Chen Guangyu-B42378
  Cc: shawn.guo, broonie, linux-arm-kernel, devicetree, linux-doc,
	linux-kernel, dmaengine, linuxppc-dev, alsa-devel, rob.herring,
	mark.rutland, swarren, pawel.moll, ijc+devicetree, s.hauer,
	Huang Shijie-B32955

Chen Guangyu-B42378 wrote:
> Without dual fifo support,  handware underrun would occasionally
> occur and then two audio channels would physically swap. This could
> be easily reproduced in low bus frequency situation, while it would
> be better if we enable dual fifo.

Ok.

ACK.

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

* Re: [PATCH 2/3] ASoC: fsl_ssi: Add dual fifo mode support
  2013-10-29 13:00       ` Timur Tabi
@ 2013-10-29 13:03         ` Chen Guangyu-B42378
  0 siblings, 0 replies; 11+ messages in thread
From: Chen Guangyu-B42378 @ 2013-10-29 13:03 UTC (permalink / raw)
  To: Timur Tabi
  Cc: shawn.guo, broonie, linux-arm-kernel, devicetree, linux-doc,
	linux-kernel, dmaengine, linuxppc-dev, alsa-devel, rob.herring,
	mark.rutland, swarren, pawel.moll, ijc+devicetree, s.hauer,
	Huang Shijie-B32955

Thank you, sir. And sorry for taking your time.

Sent by Android device.

Timur Tabi <timur@tabi.org> wrote:


Chen Guangyu-B42378 wrote:
> Without dual fifo support,  handware underrun would occasionally
> occur and then two audio channels would physically swap. This could
> be easily reproduced in low bus frequency situation, while it would
> be better if we enable dual fifo.

Ok.

ACK.



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

* Re: [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support
  2013-10-29 12:33 ` [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support Nicolin Chen
@ 2013-10-29 13:51   ` Sascha Hauer
  2013-10-30  4:48     ` Nicolin Chen
  2013-10-29 17:21   ` Kumar Gala
  1 sibling, 1 reply; 11+ messages in thread
From: Sascha Hauer @ 2013-10-29 13:51 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: timur, shawn.guo, broonie, linux-arm-kernel, devicetree,
	linux-doc, linux-kernel, dmaengine, linuxppc-dev, alsa-devel,
	rob.herring, mark.rutland, swarren, pawel.moll, ijc+devicetree,
	b32955

On Tue, Oct 29, 2013 at 08:33:15PM +0800, Nicolin Chen wrote:
> There's a script for SSI missing in current sdma script list. Thus add it.
> This script would allow SSI use its dual fifo mode to transimit/receive
> data without occasional hardware underrun/overrun.
> 
> This patch also fixed a counting error for total number of scripts.

Look at drivers/dma/imx-sdma.c:

> /**
>  * struct sdma_firmware_header - Layout of the firmware image
>  *
>  * @magic		"SDMA"
>  * @version_major	increased whenever layout of struct
>  * sdma_script_start_addrs
>  *			changes.

Can you image why this firmware has a version field? Right, it's because
it encodes the layout of struct sdma_script_start_addrs.

As the comment clearly states you have to *increase this field* when you
add scripts.

Obviously you missed that, as the firmware on lkml posted recently
shows:

> 00000000: 414d4453 00000001 00000001 0000001c SDMA............
                     ^^^^^^^^
                     Still '1'

> 00000010: 00000026 000000b4 0000067a 00000282 &.......z.......
> 00000020: ffffffff 00000000 ffffffff ffffffff ................
> 00000030: ffffffff ffffffff ffffffff ffffffff ................
> 00000040: ffffffff ffffffff 00001a6a ffffffff ........j.......
> 00000050: 000002eb 000018bb ffffffff 00000408 ................
> 00000060: ffffffff 000003c0 ffffffff ffffffff ................
> 00000070: ffffffff 000002ab ffffffff 0000037b ............{...
> 00000080: ffffffff ffffffff 0000044c 0000046e ........L...n...
> 00000090: ffffffff 00001800 ffffffff ffffffff ................
> 000000a0: 00000000 00001800 00001862 00001a16 ........b.......
                              ^^^^^^^^^^^^^^^^^
                              new script addresses introduced


> -#define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1	34
> +#define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1	37

And no, this is not a bug. It's your firmware header that is buggy.

What you need is:

#define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V2      37

You (you as a company, not you as a person) knew that it was me who
created this firmware format. So it was absolutely unnecessary to create
an incompatible firmware instead of dropping me a short note.

Please add a version check to the driver as necessary and provide a proper
firmware.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support
  2013-10-29 12:33 ` [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support Nicolin Chen
  2013-10-29 13:51   ` Sascha Hauer
@ 2013-10-29 17:21   ` Kumar Gala
  1 sibling, 0 replies; 11+ messages in thread
From: Kumar Gala @ 2013-10-29 17:21 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: timur, shawn.guo, broonie, linux-arm-kernel, devicetree,
	linux-doc, linux-kernel, dmaengine, linuxppc-dev, alsa-devel,
	rob.herring, mark.rutland, swarren, pawel.moll, ijc+devicetree,
	s.hauer, b32955


On Oct 29, 2013, at 7:33 AM, Nicolin Chen wrote:

> There's a script for SSI missing in current sdma script list. Thus add it.
> This script would allow SSI use its dual fifo mode to transimit/receive
> data without occasional hardware underrun/overrun.
> 
> This patch also fixed a counting error for total number of scripts.
> 
> Signed-off-by: Nicolin Chen <b42378@freescale.com>
> ---
> Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
> drivers/dma/imx-sdma.c                                 | 6 +++++-
> include/linux/platform_data/dma-imx-sdma.h             | 2 ++
> include/linux/platform_data/dma-imx.h                  | 1 +
> 4 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> index 4fa814d..3b933c5 100644
> --- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> +++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
> @@ -42,6 +42,7 @@ The full ID of peripheral types can be found below.
> 	19	IPU Memory
> 	20	ASRC
> 	21	ESAI
> +	22	SSI Dual FIFO
> 
> The third cell specifies the transfer priority as below.

For the DT-Binding portion:

Acked-by: Kumar Gala <galak@codeaurora.org>

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


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

* Re: [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support
  2013-10-29 13:51   ` Sascha Hauer
@ 2013-10-30  4:48     ` Nicolin Chen
  0 siblings, 0 replies; 11+ messages in thread
From: Nicolin Chen @ 2013-10-30  4:48 UTC (permalink / raw)
  To: timur, shawn.guo, broonie, linux-arm-kernel, devicetree,
	linux-doc, linux-kernel, dmaengine, linuxppc-dev, alsa-devel,
	rob.herring, mark.rutland, swarren, pawel.moll, ijc+devicetree,
	b32955

Hi Sascha,

On Tue, Oct 29, 2013 at 02:51:43PM +0100, Sascha Hauer wrote:
> Look at drivers/dma/imx-sdma.c:
> 
> > /**
> >  * struct sdma_firmware_header - Layout of the firmware image
> >  *
> >  * @magic		"SDMA"
> >  * @version_major	increased whenever layout of struct
> >  * sdma_script_start_addrs
> >  *			changes.
> 
> Can you image why this firmware has a version field? Right, it's because
> it encodes the layout of struct sdma_script_start_addrs.
> 
> As the comment clearly states you have to *increase this field* when you
> add scripts.
> 
> Obviously you missed that, as the firmware on lkml posted recently
> shows:
> 
> > 00000000: 414d4453 00000001 00000001 0000001c SDMA............
>                      ^^^^^^^^
>                      Still '1'
> 
> > 00000010: 00000026 000000b4 0000067a 00000282 &.......z.......
> > 00000020: ffffffff 00000000 ffffffff ffffffff ................
> > 00000030: ffffffff ffffffff ffffffff ffffffff ................
> > 00000040: ffffffff ffffffff 00001a6a ffffffff ........j.......
> > 00000050: 000002eb 000018bb ffffffff 00000408 ................
> > 00000060: ffffffff 000003c0 ffffffff ffffffff ................
> > 00000070: ffffffff 000002ab ffffffff 0000037b ............{...
> > 00000080: ffffffff ffffffff 0000044c 0000046e ........L...n...
> > 00000090: ffffffff 00001800 ffffffff ffffffff ................
> > 000000a0: 00000000 00001800 00001862 00001a16 ........b.......
>                               ^^^^^^^^^^^^^^^^^
>                               new script addresses introduced
> 
> 
> > -#define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1	34
> > +#define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1	37
> 
> And no, this is not a bug. It's your firmware header that is buggy.
> 

I wasn't aware that the problem is far more complicated than I thought.
And thank you for telling me all this.

> What you need is:
> 
> #define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V2      37
> 
> You (you as a company, not you as a person) knew that it was me who
> created this firmware format. So it was absolutely unnecessary to create
> an incompatible firmware instead of dropping me a short note.
> 
> Please add a version check to the driver as necessary and provide a proper
> firmware.
> 

Just currently it's not easy for me to create a new proper firmware,
and I's been told that besides this version number, it also lacks a
decent license info. So may I just refine this patch as you suggested
to add a version check and add those new scripts first?

Thank you,
Nicolin Chen

> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 



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

end of thread, other threads:[~2013-10-30  4:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-29 12:33 [PATCH 0/3] Add dual-fifo mode support of i.MX ssi Nicolin Chen
2013-10-29 12:33 ` [PATCH 1/3] dma: imx-sdma: Add ssi dual fifo script support Nicolin Chen
2013-10-29 13:51   ` Sascha Hauer
2013-10-30  4:48     ` Nicolin Chen
2013-10-29 17:21   ` Kumar Gala
2013-10-29 12:33 ` [PATCH 2/3] ASoC: fsl_ssi: Add dual fifo mode support Nicolin Chen
2013-10-29 12:42   ` Timur Tabi
2013-10-29 12:53     ` Chen Guangyu-B42378
2013-10-29 13:00       ` Timur Tabi
2013-10-29 13:03         ` Chen Guangyu-B42378
2013-10-29 12:33 ` [PATCH 3/3] ARM: dts: imx: use dual-fifo sdma script for ssi Nicolin Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).