[PATCH v7 0/9] Fix broken DMAFLUSHP on Rockchips platform

This series patches are apply on linus master branch.
Okay, this series patches verified on my github.
(https://github.com/Caesar-github/rockchip/commits/for-dma-tests)

Linux version 4.4.0+ (wxt at ubuntu)  (Ubuntu/Linaro 4.7.3-12ubuntu1) ) #14
SMP Fri Jan 22 18:13:12 CST 2016

The perious version 6 from Shawn.
(http://www.spinics.net/lists/dmaengine/msg06606.html)
I pick them up to go on upstream since Shawn hasn't much time to track them.
----

The purpose of the DMAFLUSHP instruction:
- Tell the peripheral to clear its status and control registers.
- Send a message to the peripheral to resend its level status.

There are 3 timings described in PL330 Technical Reference Manual:
- Timing 1: Burst request, can work well without DMAFLUSHP.
- Timing 2: Single and burst request, DMAC will ignore the single
            transfer request. This timing happens if there are single
            and burst request.
- Timing 3: Single transfers for a burst request, DMAC should signals
            datype to request the peripheral to flush the contents of
            any control registers. This timing happens if there is
            not enough MFIFO to places the burst data.

A peripheral may signal a DMA request during the execution of
DMAFLUSHP instruction, that cause DMA request being ignored by DMAC.

But DMAC and all peripherals on RK3X SoCs DO NOT support DMAFLUSHP.
It can't send a message to the peripheral to resend DMA request,
and the peripheral can't acknowledge a flush request from DMAC.
So all DMA requests should NOT be ignored by DMAC, and DMAC will not
notify the peripheral to flush.

To fix this problem, we need:
- Do NOT execute DMAFLUSHP instruction.
- Timing 2 and timing 3 should not happen.

Because on RK3X SoCs, there are 6 or below  channels and 32 MFIFO depth

119 more lines

From: Shawn Lin <shawn.lin@rock-chips.com>

Pl330 integrated in rk3xxx platform doesn't support
DMAFLUSHP function. So we add arm,pl330-broken-no-flushp quirk
for it.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
cc: Heiko Stuebner <heiko@sntech.de>
cc: Doug Anderson <dianders@chromium.org>
cc: Olof Johansson <redacted>
Signed-off-by: Caesar Wang <redacted>

---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1:
- rename broken-no-flushp to "arm,pl330-broken-no-flushp" suggested
  by Krzysztof.

 arch/arm/boot/dts/rk3xxx.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index 99eeea7..0012049 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi

@@ -80,6 +80,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMA1>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 		};
 
 		dmac1_ns: dma-controller at 2001c000 {

@@ -90,6 +91,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMA1>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 			status = "disabled";
 		};
 

@@ -101,6 +103,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMA2>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 		};
 	};
 

-- 
1.9.1

From: Addy Ke <redacted>

Generic dma controller on Rockchips' platform cannot support
DMAFLUSHP instruction which make dma to flush the req of non-aligned
or non-multiple of what we need. That will cause an unrecoverable
dma bus error. The saftest way is to set dma max burst to 1.

Signed-off-by: Addy ke <redacted>
Fixes: 64e36824b32b06 ("spi/rockchip: add driver for Rockchip...")
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
cc: Heiko Stuebner <heiko@sntech.de>
cc: Olof Johansson <redacted>
cc: Doug Anderson <dianders@chromium.org>
cc: Sonny Rao <redacted>

Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Caesar Wang <redacted>

---

Changes in v7: None
Changes in v6:
- remove quirks and get dma caps in order to limit burst

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 drivers/spi/spi-rockchip.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 79a8bc4..aa9561f 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c

@@ -199,6 +199,7 @@ struct rockchip_spi {
 	struct sg_table rx_sg;
 	struct rockchip_spi_dma_data dma_rx;
 	struct rockchip_spi_dma_data dma_tx;
+	struct dma_slave_caps dma_caps;
 };
 
 static inline void spi_enable_chip(struct rockchip_spi *rs, int enable)

@@ -449,7 +450,10 @@ static void rockchip_spi_prepare_dma(struct rockchip_spi *rs)
 		rxconf.direction = rs->dma_rx.direction;
 		rxconf.src_addr = rs->dma_rx.addr;
 		rxconf.src_addr_width = rs->n_bytes;
-		rxconf.src_maxburst = rs->n_bytes;
+		if (rs->dma_caps.max_burst > 4)
+			rxconf.src_maxburst = 4;
+		else
+			rxconf.src_maxburst = 1;
 		dmaengine_slave_config(rs->dma_rx.ch, &rxconf);
 
 		rxdesc = dmaengine_prep_slave_sg(

@@ -466,7 +470,10 @@ static void rockchip_spi_prepare_dma(struct rockchip_spi *rs)
 		txconf.direction = rs->dma_tx.direction;
 		txconf.dst_addr = rs->dma_tx.addr;
 		txconf.dst_addr_width = rs->n_bytes;
-		txconf.dst_maxburst = rs->n_bytes;
+		if (rs->dma_caps.max_burst > 4)
+			txconf.dst_maxburst = 4;
+		else
+			txconf.dst_maxburst = 1;
 		dmaengine_slave_config(rs->dma_tx.ch, &txconf);
 
 		txdesc = dmaengine_prep_slave_sg(

@@ -730,6 +737,7 @@ static int rockchip_spi_probe(struct platform_device *pdev)
 	}
 
 	if (rs->dma_tx.ch && rs->dma_rx.ch) {
+		dma_get_slave_caps(rs->dma_rx.ch, &(rs->dma_caps));
 		rs->dma_tx.addr = (dma_addr_t)(mem->start + ROCKCHIP_SPI_TXDR);
 		rs->dma_rx.addr = (dma_addr_t)(mem->start + ROCKCHIP_SPI_RXDR);
 		rs->dma_tx.direction = DMA_MEM_TO_DEV;

-- 
1.9.1

Pl330 integrated in rk3036 platform that doesn't support
DMAFLUSHP function. So we add 'arm,pl330-broken-no-flushp' quirk
for rk3036.

Signed-off-by: Caesar Wang <redacted>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 arch/arm/boot/dts/rk3036.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index ee457a2..242cafc 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi

@@ -102,6 +102,7 @@
 		pdma: pdma at 20078000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x20078000 0x4000>;
+			arm,pl330-broken-no-flushp;
 			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
 			#dma-cells = <1>;

-- 
1.9.1

From: Addy Ke <redacted>

Pl330 integrated in rk3288 platform doesn't support
DMAFLUSHP function. So we add arm,pl330-broken-no-flushp quirk
for it.

Signed-off-by: Addy Ke <redacted>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
cc: Heiko Stuebner <heiko@sntech.de>
cc: Olof Johansson <redacted>
cc: Sonny Rao <redacted>

Reviewed-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Sonny Rao <redacted>
Signed-off-by: Caesar Wang <redacted>

---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3:
- add Reviewed-by: Sonny Rao [off-list ref]

Changes in v2:
- amend the author
- add Reviewed-by: Doug Anderson [off-list ref]
- amend Olof's mail address

Changes in v1:
- rename broken-no-flushp to "arm,pl330-broken-no-flushp" suggested
  by Krzysztof.
  - remove Sunny's tag

 arch/arm/boot/dts/rk3288.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index f0a0919..eba40dc 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi

@@ -147,6 +147,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMAC2>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 		};
 
 		dmac_bus_ns: dma-controller at ff600000 {

@@ -158,6 +159,7 @@
 			clocks = <&cru ACLK_DMAC1>;
 			clock-names = "apb_pclk";
 			status = "disabled";
+			arm,pl330-broken-no-flushp;
 		};
 
 		dmac_bus_s: dma-controller at ffb20000 {

@@ -168,6 +170,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMAC1>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 		};
 	};
 

-- 
1.9.1

Hello Vinod,

Is there any chance apply the DMA patches?:-)


-
Caesar

? 2016?01?22? 19:06, Caesar Wang ??:

This series patches are apply on linus master branch.
Okay, this series patches verified on my github.
(https://github.com/Caesar-github/rockchip/commits/for-dma-tests)

Linux version 4.4.0+ (wxt at ubuntu)  (Ubuntu/Linaro 4.7.3-12ubuntu1) ) #14
SMP Fri Jan 22 18:13:12 CST 2016

The perious version 6 from Shawn.
(http://www.spinics.net/lists/dmaengine/msg06606.html)
I pick them up to go on upstream since Shawn hasn't much time to track them.
----

The purpose of the DMAFLUSHP instruction:
- Tell the peripheral to clear its status and control registers.
- Send a message to the peripheral to resend its level status.

There are 3 timings described in PL330 Technical Reference Manual:
- Timing 1: Burst request, can work well without DMAFLUSHP.
- Timing 2: Single and burst request, DMAC will ignore the single
             transfer request. This timing happens if there are single
             and burst request.
- Timing 3: Single transfers for a burst request, DMAC should signals
             datype to request the peripheral to flush the contents of
             any control registers. This timing happens if there is
             not enough MFIFO to places the burst data.

A peripheral may signal a DMA request during the execution of
DMAFLUSHP instruction, that cause DMA request being ignored by DMAC.

But DMAC and all peripherals on RK3X SoCs DO NOT support DMAFLUSHP.
It can't send a message to the peripheral to resend DMA request,
and the peripheral can't acknowledge a flush request from DMAC.
So all DMA requests should NOT be ignored by DMAC, and DMAC will not
notify the peripheral to flush.

To fix this problem, we need:
- Do NOT execute DMAFLUSHP instruction.
- Timing 2 and timing 3 should not happen.

Because on RK3X SoCs, there are 6 or below  channels and 32 MFIFO depth

117 more quoted lines

-- 
Thanks,
Caesar

On Mon, Feb 01, 2016 at 02:56:54PM +0800, Caesar Wang wrote:

Hello Vinod,

Is there any chance apply the DMA patches?:-)

I was waiting for any ACKs on ARM patches before I merge this series.

? 2016?01?22? 19:06, Caesar Wang ??:

quoted

This series patches are apply on linus master branch.
Okay, this series patches verified on my github.
(https://github.com/Caesar-github/rockchip/commits/for-dma-tests)

Linux version 4.4.0+ (wxt at ubuntu)  (Ubuntu/Linaro 4.7.3-12ubuntu1) ) #14
SMP Fri Jan 22 18:13:12 CST 2016

The perious version 6 from Shawn.
(http://www.spinics.net/lists/dmaengine/msg06606.html)
I pick them up to go on upstream since Shawn hasn't much time to track them.
----

The purpose of the DMAFLUSHP instruction:
- Tell the peripheral to clear its status and control registers.
- Send a message to the peripheral to resend its level status.

There are 3 timings described in PL330 Technical Reference Manual:
- Timing 1: Burst request, can work well without DMAFLUSHP.
- Timing 2: Single and burst request, DMAC will ignore the single
            transfer request. This timing happens if there are single
            and burst request.
- Timing 3: Single transfers for a burst request, DMAC should signals
            datype to request the peripheral to flush the contents of
            any control registers. This timing happens if there is
            not enough MFIFO to places the burst data.

A peripheral may signal a DMA request during the execution of
DMAFLUSHP instruction, that cause DMA request being ignored by DMAC.

But DMAC and all peripherals on RK3X SoCs DO NOT support DMAFLUSHP.
It can't send a message to the peripheral to resend DMA request,
and the peripheral can't acknowledge a flush request from DMAC.
So all DMA requests should NOT be ignored by DMAC, and DMAC will not
notify the peripheral to flush.

To fix this problem, we need:
- Do NOT execute DMAFLUSHP instruction.
- Timing 2 and timing 3 should not happen.

124 more quoted lines

-- 
~Vinod

Hi Vinod,

Am Montag, 8. Februar 2016, 08:41:34 schrieb Vinod Koul:

On Mon, Feb 01, 2016 at 02:56:54PM +0800, Caesar Wang wrote:

quoted

Hello Vinod,

Is there any chance apply the DMA patches?:-)

I was waiting for any ACKs on ARM patches before I merge this series.

I think the more regular way is for the driver maintainer to take the 
driver-specific stuff and the devicetree parts going through the specific arm-
tree. But if you really want to take the whole series, for patches 4,5,6

- ARM: dts: Add arm, pl330-broken-no-flushp quirk for rk3288 platform
- ARM: dts: Add arm, pl330-broken-no-flushp quirk for rk3xxx platform
- ARM: dts: add pl330-broken-no-flushp quirk for rk3036 SoCs

Acked-by: Heiko Stuebner <heiko@sntech.de>


Heiko

Hi Heiko,

On Mon, Feb 08, 2016 at 10:27:04AM +0100, Heiko Stuebner wrote:

Hi Vinod,

Am Montag, 8. Februar 2016, 08:41:34 schrieb Vinod Koul:

quoted

On Mon, Feb 01, 2016 at 02:56:54PM +0800, Caesar Wang wrote:

quoted

Hello Vinod,

Is there any chance apply the DMA patches?:-)

I was waiting for any ACKs on ARM patches before I merge this series.

I think the more regular way is for the driver maintainer to take the 
driver-specific stuff and the devicetree parts going through the specific arm-
tree. But if you really want to take the whole series, for patches 4,5,6

Yes but that causes cross tree dependencies, which looking at this won't be
a big problem, so I can safely merge dmaengine changes and rest can go thru
ARM tree.

Typically submitter should say which tree he/she prefers, which was not
provided..

So, are all okay to merge the entire series thru dmaengine tree or
independent. I don't mind either

- ARM: dts: Add arm, pl330-broken-no-flushp quirk for rk3288 platform
- ARM: dts: Add arm, pl330-broken-no-flushp quirk for rk3xxx platform
- ARM: dts: add pl330-broken-no-flushp quirk for rk3036 SoCs

Acked-by: Heiko Stuebner <heiko@sntech.de>

Thanks
-- 
~Vinod

On Mon, Feb 08, 2016 at 06:44:19PM +0530, Vinod Koul wrote:

On Mon, Feb 08, 2016 at 10:27:04AM +0100, Heiko Stuebner wrote:

quoted

I think the more regular way is for the driver maintainer to take the 
driver-specific stuff and the devicetree parts going through the specific arm-
tree. But if you really want to take the whole series, for patches 4,5,6

Yes but that causes cross tree dependencies, which looking at this won't be
a big problem, so I can safely merge dmaengine changes and rest can go thru
ARM tree.

Not for the DT stuff - since the DT is supposed to be a stable ABI there
really shouldn't be any hard dependencies on things outside the DT.

Typically submitter should say which tree he/she prefers, which was not
provided..

It's the norm for DT stuff.
-------------- 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/20160208/42079e83/attachment.sig>

Hi Vinod,

Am Montag, 8. Februar 2016, 18:44:19 schrieb Vinod Koul:

On Mon, Feb 08, 2016 at 10:27:04AM +0100, Heiko Stuebner wrote:

quoted

Am Montag, 8. Februar 2016, 08:41:34 schrieb Vinod Koul:

quoted

On Mon, Feb 01, 2016 at 02:56:54PM +0800, Caesar Wang wrote:

quoted

Hello Vinod,

Is there any chance apply the DMA patches?:-)

I was waiting for any ACKs on ARM patches before I merge this series.

I think the more regular way is for the driver maintainer to take the
driver-specific stuff and the devicetree parts going through the
specific arm- tree. But if you really want to take the whole series,
for patches 4,5,6

Yes but that causes cross tree dependencies, which looking at this won't
be a big problem, so I can safely merge dmaengine changes and rest can go
thru ARM tree.

Typically submitter should say which tree he/she prefers, which was not
provided..

So, are all okay to merge the entire series thru dmaengine tree or
independent. I don't mind either

In general I don't mind either solution, but would prefer me taking the 3 
"ARM: dts: foo" patches, while the rest would go through your tree.

Main reason is to prevent any possible conflicts with other Rockchip 
devicetree additions also going into 4.6 .


Thanks
Heiko

quoted

- ARM: dts: Add arm, pl330-broken-no-flushp quirk for rk3288 platform
- ARM: dts: Add arm, pl330-broken-no-flushp quirk for rk3xxx platform
- ARM: dts: add pl330-broken-no-flushp quirk for rk3036 SoCs

Acked-by: Heiko Stuebner <heiko@sntech.de>

Thanks

On Mon, Feb 08, 2016 at 11:21:57PM +0100, Heiko Stuebner wrote:

Hi Vinod,

Am Montag, 8. Februar 2016, 18:44:19 schrieb Vinod Koul:

quoted

On Mon, Feb 08, 2016 at 10:27:04AM +0100, Heiko Stuebner wrote:

quoted

Am Montag, 8. Februar 2016, 08:41:34 schrieb Vinod Koul:

quoted

On Mon, Feb 01, 2016 at 02:56:54PM +0800, Caesar Wang wrote:

quoted

Hello Vinod,

Is there any chance apply the DMA patches?:-)

I was waiting for any ACKs on ARM patches before I merge this series.

I think the more regular way is for the driver maintainer to take the
driver-specific stuff and the devicetree parts going through the
specific arm- tree. But if you really want to take the whole series,
for patches 4,5,6

Yes but that causes cross tree dependencies, which looking at this won't
be a big problem, so I can safely merge dmaengine changes and rest can go
thru ARM tree.

Typically submitter should say which tree he/she prefers, which was not
provided..

So, are all okay to merge the entire series thru dmaengine tree or
independent. I don't mind either

In general I don't mind either solution, but would prefer me taking the 3 
"ARM: dts: foo" patches, while the rest would go through your tree.

Main reason is to prevent any possible conflicts with other Rockchip 
devicetree additions also going into 4.6 .

Sounds good, will queue them up

-- 
~Vinod

On Fri, Jan 22, 2016 at 07:06:52PM +0800, Caesar Wang wrote:

From: Addy Ke <redacted>

Generic dma controller on Rockchips' platform cannot support
DMAFLUSHP instruction which make dma to flush the req of non-aligned
or non-multiple of what we need. That will cause an unrecoverable
dma bus error. The saftest way is to set dma max burst to 1.

Applied, thanks

-- 
~Vinod

Am Freitag, 22. Januar 2016, 19:06:49 schrieb Caesar Wang:

Pl330 integrated in rk3036 platform that doesn't support
DMAFLUSHP function. So we add 'arm,pl330-broken-no-flushp' quirk
for rk3036.

Signed-off-by: Caesar Wang <redacted>

applied the 3 (4/9, 5/9, 6/9) dts patches to my dts32 branch for 4.6
(after fixing the patch subjects to match the general style-convention)