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

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

[Shawn Lin]

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
for DMAC_BUS, and 8 channels and 64 MFIFO depth for DMAC_PERI, it is
impossible to hit the timing 3 if burst length is equal or less than 4.

Since the request type signal by the peripheral can only be set by
software. We can set Rockchip Soc's GRF_PERIDMAC_CON0[2:1] to select single
or burst request, if it is set b01,  all of the peripharals will signal a brust
request. So the timing 2 will not happen, too.

So DMAC on RK3X can support single or burst transfer, but can't support
mixed transfer.

Because burst transfer is more efficient than single transfer, this is
confirmed by our ASIC team, who strongly suggest to use burst transfer.
And this is confirmed by Addy's test on RK3288-Pink2 board, the speed of
spi flash burst transfer will increase about two times than single transfer.
Also, I have tested dw_mmc with pl330 on RK3188 platform to double confirm
the result. That means burst transfer is reansonable.

So we need a quirk not to execute DMAFLUSHP instruction and to use burst
transfer.

Note:
- The Rockchip Soc default value of GRF_PERIDMAC_CON0[2:1] is b01. To
  support brust transfer, these bits should not be changed in bootloader.


Changes in v3:
- add more rockchip drivers' changes in this patchset
- add Reviewed-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Changes in v2:
- amend the author
- reorder the patches suggested by Doug
- add Reviewed-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> for
  rk3288.dtsi patch and arm-pl330.txt patch
- amend Olof's mail address

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

Addy Ke (3):
  DMA: pl330: add quirk for broken no flushp
  ARM: dts: Add arm,pl330-broken-no-flushp quirk for rk3288 platform
  spi: rockchip: modify DMA max burst to 1

Boojin Kim (1):
  DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit

Shawn Lin (4):
  Documentation: arm-pl330: add description of
    arm,pl330-broken-no-flushp
  Documentation: spi-rockchip: add description of
    rockchip,spi-broken-burst-len
  Documentation: rockchip-i2s: add description of
    rockchip,i2s-broken-burst-len
  ARM: dts: Add arm,pl330-broken-no-flushp quirk for rk3xxx platform

Yiwei Cai (1):
  ASoC: rockchip_i2s: modify DMA max burst to 1

 .../devicetree/bindings/dma/arm-pl330.txt          |   1 +
 .../devicetree/bindings/sound/rockchip-i2s.txt     |   4 +
 .../devicetree/bindings/spi/spi-rockchip.txt       |   3 +-
 arch/arm/boot/dts/rk3288.dtsi                      |   7 ++
 arch/arm/boot/dts/rk3xxx.dtsi                      |   5 +
 drivers/dma/pl330.c                                | 101 +++++++++++++++------
 drivers/spi/spi-rockchip.c                         |  16 +++-
 sound/soc/rockchip/rockchip_i2s.c                  |  10 +-
 8 files changed, 113 insertions(+), 34 deletions(-)

-- 
2.3.7


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

[PATCH v3 1/9] DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit

[Shawn Lin]

From: Boojin Kim <boojin.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

This patch adds to support burst mode for dev-to-mem and
mem-to-dev transmit.

Signed-off-by: Boojin Kim <boojin.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Addy Ke <addy.ke-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
cc: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>

Reviewed-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v3:
- add Sunny's tag
- add more rockchip drivers' changes in this patchset

Changes in v2:
- amend the author
- reorder the patches suggested by Doug
- add Reviewed-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> for
  rk3288.dtsi patch and arm-pl330.txt patch

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

 drivers/dma/pl330.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index ecab4ea0..0d544d2 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -1141,10 +1141,13 @@ static inline int _ldst_devtomem(unsigned dry_run, u8 buf[],
 		const struct _xfer_spec *pxs, int cyc)
 {
 	int off = 0;
+	enum pl330_cond cond;
+
+	cond = (pxs->desc->rqcfg.brst_len == 1) ? SINGLE : BURST;
 
 	while (cyc--) {
-		off += _emit_WFP(dry_run, &buf[off], SINGLE, pxs->desc->peri);
-		off += _emit_LDP(dry_run, &buf[off], SINGLE, pxs->desc->peri);
+		off += _emit_WFP(dry_run, &buf[off], cond, pxs->desc->peri);
+		off += _emit_LDP(dry_run, &buf[off], cond, pxs->desc->peri);
 		off += _emit_ST(dry_run, &buf[off], ALWAYS);
 		off += _emit_FLUSHP(dry_run, &buf[off], pxs->desc->peri);
 	}
@@ -1156,11 +1159,14 @@ static inline int _ldst_memtodev(unsigned dry_run, u8 buf[],
 		const struct _xfer_spec *pxs, int cyc)
 {
 	int off = 0;
+	enum pl330_cond cond;
+
+	cond = (pxs->desc->rqcfg.brst_len == 1) ? SINGLE : BURST;
 
 	while (cyc--) {
-		off += _emit_WFP(dry_run, &buf[off], SINGLE, pxs->desc->peri);
+		off += _emit_WFP(dry_run, &buf[off], cond, pxs->desc->peri);
 		off += _emit_LD(dry_run, &buf[off], ALWAYS);
-		off += _emit_STP(dry_run, &buf[off], SINGLE, pxs->desc->peri);
+		off += _emit_STP(dry_run, &buf[off], cond, pxs->desc->peri);
 		off += _emit_FLUSHP(dry_run, &buf[off], pxs->desc->peri);
 	}
 
@@ -2557,7 +2563,7 @@ static struct dma_async_tx_descriptor *pl330_prep_dma_cyclic(
 
 		desc->rqtype = direction;
 		desc->rqcfg.brst_size = pch->burst_sz;
-		desc->rqcfg.brst_len = 1;
+		desc->rqcfg.brst_len = pch->burst_len;
 		desc->bytes_requested = period_len;
 		fill_px(&desc->px, dst, src, period_len);
 
@@ -2702,7 +2708,7 @@ pl330_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 		}
 
 		desc->rqcfg.brst_size = pch->burst_sz;
-		desc->rqcfg.brst_len = 1;
+		desc->rqcfg.brst_len = pch->burst_len;
 		desc->rqtype = direction;
 		desc->bytes_requested = sg_dma_len(sg);
 	}
-- 
2.3.7


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

[PATCH v3 3/9] Documentation: spi-rockchip: add description of rockchip,spi-broken-burst-len

[Shawn Lin]

Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
cc: Addy Ke <addy.ke-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
cc: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
cc: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v3: None
Changes in v2: None
Changes in v1: None

 Documentation/devicetree/bindings/spi/spi-rockchip.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-rockchip.txt b/Documentation/devicetree/bindings/spi/spi-rockchip.txt
index 0c491bd..2256610 100644
--- a/Documentation/devicetree/bindings/spi/spi-rockchip.txt
+++ b/Documentation/devicetree/bindings/spi/spi-rockchip.txt
@@ -27,7 +27,8 @@ Optional Properties:
 - rx-sample-delay-ns: nanoseconds to delay after the SCLK edge before sampling
 		Rx data (may need to be fine tuned for high capacitance lines).
 		No delay (0) by default.
-
+- rockchip,spi-broken-burst-len: quirk for broken burst len on Rockchip platform.
+			Add this property will force driver to set burst ONE.
 
 Example:
 
-- 
2.3.7


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

[PATCH v3 9/9] ASoC: rockchip_i2s: modify DMA max burst to 1

[Shawn Lin]

From: Yiwei Cai <cain.cai-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Test with command -
arecord -D hw:0,0 /tmp/a.wav, there are the error dump:
[  134.899396] dma-pl330 ffb20000.dma-controller: fill_queue:2251 Bad Desc(7)

This error is happening when no a multiple of burst size * burst length are
coming in. The root cause is pl330 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 set before. The saftest way is to set dma max burst to 1.

Signed-off-by: Yiwei Cai <cain.cai-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Fixes: 4495c89fc ("ASoC: add driver for Rockchip RK3xxx I2S")
Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
cc: Addy Ke <addy.ke-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
cc: Jianqun Xu <xjq-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
cc: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
cc: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
cc: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---

Changes in v3: None
Changes in v2: None
Changes in v1: None

 sound/soc/rockchip/rockchip_i2s.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index acb5be5..5d35b2c 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -457,13 +457,19 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
 		return PTR_ERR(i2s->regmap);
 	}
 
+	if (of_property_read_bool(np, "rockchip,i2s-broken-burst-len")) {
+		i2s->playback_dma_data.maxburst = 1;
+		i2s->capture_dma_data.maxburst = 1;
+	} else {
+		i2s->playback_dma_data.maxburst = 4;
+		i2s->capture_dma_data.maxburst = 4;
+	}
+
 	i2s->playback_dma_data.addr = res->start + I2S_TXDR;
 	i2s->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-	i2s->playback_dma_data.maxburst = 4;
 
 	i2s->capture_dma_data.addr = res->start + I2S_RXDR;
 	i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-	i2s->capture_dma_data.maxburst = 4;
 
 	i2s->dev = &pdev->dev;
 	dev_set_drvdata(&pdev->dev, i2s);
-- 
2.3.7


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

[PATCH v3 2/9] Documentation: arm-pl330: add description of arm,pl330-broken-no-flushp

[Shawn Lin]

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

Reviewed-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
---

Changes in v3:
- add Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

Changes in v2:
- add Reviewed-by: Doug Anderson <dianders@chromium.org>

Changes in v1:
- rename broken-no-flushp to "arm,pl330-broken-no-flushp" suggested
  by Krzysztof.

 Documentation/devicetree/bindings/dma/arm-pl330.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/dma/arm-pl330.txt b/Documentation/devicetree/bindings/dma/arm-pl330.txt
index 2675658..db7e226 100644
--- a/Documentation/devicetree/bindings/dma/arm-pl330.txt
+++ b/Documentation/devicetree/bindings/dma/arm-pl330.txt
@@ -15,6 +15,7 @@ Optional properties:
     cells in the dmas property of client device.
   - dma-channels: contains the total number of DMA channels supported by the DMAC
   - dma-requests: contains the total number of DMA requests supported by the DMAC
+  - arm,pl330-broken-no-flushp: quirk for avoiding to execute DMAFLUSHP
 
 Example:
 
-- 
2.3.7

[PATCH v3 4/9] Documentation: rockchip-i2s: add description of rockchip,i2s-broken-burst-len

[Shawn Lin]

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
cc: Addy Ke <addy.ke@rock-chips.com>
cc: Jianqun Xu <xjq@rock-chips.com>
cc: Heiko Stuebner <heiko@sntech.de>
cc: Doug Anderson <dianders@chromium.org>
cc: Olof Johansson <olof@lixom.net>
cc: Sonny Rao <sonnyrao@chromium.org>
---

Changes in v3: None
Changes in v2: None
Changes in v1: None

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

diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.txt b/Documentation/devicetree/bindings/sound/rockchip-i2s.txt
index 9b82c20..9987787 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,i2s-broken-burst-len: quirk for broken burst len on Rockchip platform.
+				 Add this property will force driver to set burst ONE.
+
 Example for rk3288 I2S controller:
 
 i2s@ff890000 {
-- 
2.3.7

[PATCH v3 5/9] DMA: pl330: add quirk for broken no flushp

[Shawn Lin]

From: Addy Ke <addy.ke@rock-chips.com>

This patch add "arm,pl330-broken-no-flushp" quirk to avoid execute
DMAFLUSHP if Soc doesn't support it.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
cc: Doug Anderson <dianders@chromium.org>
cc: Heiko Stuebner <heiko@sntech.de>
cc: Olof Johansson <olof@lixom.net>

Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
---

Changes in v3:
- add Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

Changes in v2:
- amend the author
- fix Olof's mail address

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

 drivers/dma/pl330.c | 87 ++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 25 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 0d544d2..3b9b426 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -34,6 +34,8 @@
 #define PL330_MAX_IRQS		32
 #define PL330_MAX_PERI		32
 
+#define PL330_QUIRK_BROKEN_NO_FLUSHP BIT(0)
+
 enum pl330_cachectrl {
 	CCTRL0,		/* Noncacheable and nonbufferable */
 	CCTRL1,		/* Bufferable only */
@@ -488,6 +490,17 @@ struct pl330_dmac {
 	/* Peripheral channels connected to this DMAC */
 	unsigned int num_peripherals;
 	struct dma_pl330_chan *peripherals; /* keep at end */
+	int quirks;
+};
+
+static struct pl330_of_quirks {
+	char *quirk;
+	int id;
+} of_quirks[] = {
+	{
+		.quirk = "arm,pl330-broken-no-flushp",
+		.id = PL330_QUIRK_BROKEN_NO_FLUSHP,
+	}
 };
 
 struct dma_pl330_desc {
@@ -1137,53 +1150,68 @@ static inline int _ldst_memtomem(unsigned dry_run, u8 buf[],
 	return off;
 }
 
-static inline int _ldst_devtomem(unsigned dry_run, u8 buf[],
-		const struct _xfer_spec *pxs, int cyc)
+static inline int _ldst_devtomem(struct pl330_dmac *pl330, unsigned dry_run,
+				 u8 buf[], const struct _xfer_spec *pxs,
+				 int cyc)
 {
 	int off = 0;
 	enum pl330_cond cond;
 
-	cond = (pxs->desc->rqcfg.brst_len == 1) ? SINGLE : BURST;
+	if (pl330->quirks & PL330_QUIRK_BROKEN_NO_FLUSHP)
+		cond = BURST;
+	else
+		cond = (pxs->desc->rqcfg.brst_len == 1) ? SINGLE : BURST;
 
 	while (cyc--) {
 		off += _emit_WFP(dry_run, &buf[off], cond, pxs->desc->peri);
 		off += _emit_LDP(dry_run, &buf[off], cond, pxs->desc->peri);
 		off += _emit_ST(dry_run, &buf[off], ALWAYS);
-		off += _emit_FLUSHP(dry_run, &buf[off], pxs->desc->peri);
+
+		if (!(pl330->quirks & PL330_QUIRK_BROKEN_NO_FLUSHP))
+			off += _emit_FLUSHP(dry_run, &buf[off],
+					    pxs->desc->peri);
 	}
 
 	return off;
 }
 
-static inline int _ldst_memtodev(unsigned dry_run, u8 buf[],
-		const struct _xfer_spec *pxs, int cyc)
+static inline int _ldst_memtodev(struct pl330_dmac *pl330,
+				 unsigned dry_run, u8 buf[],
+				 const struct _xfer_spec *pxs, int cyc)
 {
 	int off = 0;
 	enum pl330_cond cond;
 
-	cond = (pxs->desc->rqcfg.brst_len == 1) ? SINGLE : BURST;
+	if (pl330->quirks & PL330_QUIRK_BROKEN_NO_FLUSHP)
+		cond = BURST;
+	else
+		cond = (pxs->desc->rqcfg.brst_len == 1) ? SINGLE : BURST;
+
 
 	while (cyc--) {
 		off += _emit_WFP(dry_run, &buf[off], cond, pxs->desc->peri);
 		off += _emit_LD(dry_run, &buf[off], ALWAYS);
 		off += _emit_STP(dry_run, &buf[off], cond, pxs->desc->peri);
-		off += _emit_FLUSHP(dry_run, &buf[off], pxs->desc->peri);
+
+		if (!(pl330->quirks & PL330_QUIRK_BROKEN_NO_FLUSHP))
+			off += _emit_FLUSHP(dry_run, &buf[off],
+					    pxs->desc->peri);
 	}
 
 	return off;
 }
 
-static int _bursts(unsigned dry_run, u8 buf[],
+static int _bursts(struct pl330_dmac *pl330, unsigned dry_run, u8 buf[],
 		const struct _xfer_spec *pxs, int cyc)
 {
 	int off = 0;
 
 	switch (pxs->desc->rqtype) {
 	case DMA_MEM_TO_DEV:
-		off += _ldst_memtodev(dry_run, &buf[off], pxs, cyc);
+		off += _ldst_memtodev(pl330, dry_run, &buf[off], pxs, cyc);
 		break;
 	case DMA_DEV_TO_MEM:
-		off += _ldst_devtomem(dry_run, &buf[off], pxs, cyc);
+		off += _ldst_devtomem(pl330, dry_run, &buf[off], pxs, cyc);
 		break;
 	case DMA_MEM_TO_MEM:
 		off += _ldst_memtomem(dry_run, &buf[off], pxs, cyc);
@@ -1197,7 +1225,7 @@ static int _bursts(unsigned dry_run, u8 buf[],
 }
 
 /* Returns bytes consumed and updates bursts */
-static inline int _loop(unsigned dry_run, u8 buf[],
+static inline int _loop(struct pl330_dmac *pl330, unsigned dry_run, u8 buf[],
 		unsigned long *bursts, const struct _xfer_spec *pxs)
 {
 	int cyc, cycmax, szlp, szlpend, szbrst, off;
@@ -1220,7 +1248,7 @@ static inline int _loop(unsigned dry_run, u8 buf[],
 	}
 
 	szlp = _emit_LP(1, buf, 0, 0);
-	szbrst = _bursts(1, buf, pxs, 1);
+	szbrst = _bursts(pl330, 1, buf, pxs, 1);
 
 	lpend.cond = ALWAYS;
 	lpend.forever = false;
@@ -1252,7 +1280,7 @@ static inline int _loop(unsigned dry_run, u8 buf[],
 	off += _emit_LP(dry_run, &buf[off], 1, lcnt1);
 	ljmp1 = off;
 
-	off += _bursts(dry_run, &buf[off], pxs, cyc);
+	off += _bursts(pl330, dry_run, &buf[off], pxs, cyc);
 
 	lpend.cond = ALWAYS;
 	lpend.forever = false;
@@ -1275,8 +1303,9 @@ static inline int _loop(unsigned dry_run, u8 buf[],
 	return off;
 }
 
-static inline int _setup_loops(unsigned dry_run, u8 buf[],
-		const struct _xfer_spec *pxs)
+static inline int _setup_loops(struct pl330_dmac *pl330,
+			       unsigned dry_run, u8 buf[],
+			       const struct _xfer_spec *pxs)
 {
 	struct pl330_xfer *x = &pxs->desc->px;
 	u32 ccr = pxs->ccr;
@@ -1285,15 +1314,16 @@ static inline int _setup_loops(unsigned dry_run, u8 buf[],
 
 	while (bursts) {
 		c = bursts;
-		off += _loop(dry_run, &buf[off], &c, pxs);
+		off += _loop(pl330, dry_run, &buf[off], &c, pxs);
 		bursts -= c;
 	}
 
 	return off;
 }
 
-static inline int _setup_xfer(unsigned dry_run, u8 buf[],
-		const struct _xfer_spec *pxs)
+static inline int _setup_xfer(struct pl330_dmac *pl330,
+			      unsigned dry_run, u8 buf[],
+			      const struct _xfer_spec *pxs)
 {
 	struct pl330_xfer *x = &pxs->desc->px;
 	int off = 0;
@@ -1304,7 +1334,7 @@ static inline int _setup_xfer(unsigned dry_run, u8 buf[],
 	off += _emit_MOV(dry_run, &buf[off], DAR, x->dst_addr);
 
 	/* Setup Loop(s) */
-	off += _setup_loops(dry_run, &buf[off], pxs);
+	off += _setup_loops(pl330, dry_run, &buf[off], pxs);
 
 	return off;
 }
@@ -1313,8 +1343,9 @@ static inline int _setup_xfer(unsigned dry_run, u8 buf[],
  * A req is a sequence of one or more xfer units.
  * Returns the number of bytes taken to setup the MC for the req.
  */
-static int _setup_req(unsigned dry_run, struct pl330_thread *thrd,
-		unsigned index, struct _xfer_spec *pxs)
+static int _setup_req(struct pl330_dmac *pl330, unsigned dry_run,
+		      struct pl330_thread *thrd, unsigned index,
+		      struct _xfer_spec *pxs)
 {
 	struct _pl330_req *req = &thrd->req[index];
 	struct pl330_xfer *x;
@@ -1331,7 +1362,7 @@ static int _setup_req(unsigned dry_run, struct pl330_thread *thrd,
 	if (x->bytes % (BRST_SIZE(pxs->ccr) * BRST_LEN(pxs->ccr)))
 		return -EINVAL;
 
-	off += _setup_xfer(dry_run, &buf[off], pxs);
+	off += _setup_xfer(pl330, dry_run, &buf[off], pxs);
 
 	/* DMASEV peripheral/event */
 	off += _emit_SEV(dry_run, &buf[off], thrd->ev);
@@ -1425,7 +1456,7 @@ static int pl330_submit_req(struct pl330_thread *thrd,
 	xs.desc = desc;
 
 	/* First dry run to check if req is acceptable */
-	ret = _setup_req(1, thrd, idx, &xs);
+	ret = _setup_req(pl330, 1, thrd, idx, &xs);
 	if (ret < 0)
 		goto xfer_exit;
 
@@ -1439,7 +1470,7 @@ static int pl330_submit_req(struct pl330_thread *thrd,
 	/* Hook the request */
 	thrd->lstenq = idx;
 	thrd->req[idx].desc = desc;
-	_setup_req(0, thrd, idx, &xs);
+	_setup_req(pl330, 0, thrd, idx, &xs);
 
 	ret = 0;
 
@@ -2784,6 +2815,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 	struct resource *res;
 	int i, ret, irq;
 	int num_chan;
+	struct device_node *np = adev->dev.of_node;
 
 	pdat = dev_get_platdata(&adev->dev);
 
@@ -2803,6 +2835,11 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 
 	pl330->mcbufsz = pdat ? pdat->mcbuf_sz : 0;
 
+	/* get quirk */
+	for (i = 0; i < ARRAY_SIZE(of_quirks); i++)
+		if (of_property_read_bool(np, of_quirks[i].quirk))
+			pl330->quirks |= of_quirks[i].id;
+
 	res = &adev->res;
 	pl330->base = devm_ioremap_resource(&adev->dev, res);
 	if (IS_ERR(pl330->base))
-- 
2.3.7

[PATCH v3 6/9] ARM: dts: Add arm,pl330-broken-no-flushp quirk for rk3288 platform

[Shawn Lin]

From: Addy Ke <addy.ke@rock-chips.com>

Pl330 integrated in rk3288 platform doesn't support
DMAFLUSHP function. So we add arm,pl330-broken-no-flushp quirk
for it. Also we add rockchip,i2s-broken-burst-len and
rockchip,spi-broken-burst-len to fix this problem.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
cc: Heiko Stuebner <heiko@sntech.de>
cc: Olof Johansson <olof@lixom.net>
cc: Sonny Rao <sonnyrao@chromium.org>

Reviewed-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
---

Changes in v3:
- add Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

Changes in v2:
- amend the author
- add Reviewed-by: Doug Anderson <dianders@chromium.org>
- 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 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 22316d0..2654167 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -144,6 +144,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMAC2>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 		};
 
 		dmac_bus_ns: dma-controller@ff600000 {
@@ -155,6 +156,7 @@
 			clocks = <&cru ACLK_DMAC1>;
 			clock-names = "apb_pclk";
 			status = "disabled";
+			arm,pl330-broken-no-flushp;
 		};
 
 		dmac_bus_s: dma-controller@ffb20000 {
@@ -165,6 +167,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMAC1>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 		};
 	};
 
@@ -264,6 +267,7 @@
 		reg = <0xff110000 0x1000>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		rockchip,spi-broken-burst-len;
 		status = "disabled";
 	};
 
@@ -279,6 +283,7 @@
 		reg = <0xff120000 0x1000>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		rockchip,spi-broken-burst-len;
 		status = "disabled";
 	};
 
@@ -294,6 +299,7 @@
 		reg = <0xff130000 0x1000>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		rockchip,spi-broken-burst-len;
 		status = "disabled";
 	};
 
@@ -642,6 +648,7 @@
 		clocks = <&cru HCLK_I2S0>, <&cru SCLK_I2S0>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s0_bus>;
+		rockchip,i2s-broken-burst-len;
 		status = "disabled";
 	};
 
-- 
2.3.7

[PATCH v3 7/9] ARM: dts: Add arm,pl330-broken-no-flushp quirk for rk3xxx platform

[Shawn Lin]

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

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 <olof@lixom.net>

---

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 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index a2ae9f3..ae15a81 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -79,6 +79,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMA1>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 		};
 
 		dmac1_ns: dma-controller@2001c000 {
@@ -89,6 +90,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMA1>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 			status = "disabled";
 		};
 
@@ -100,6 +102,7 @@
 			#dma-cells = <1>;
 			clocks = <&cru ACLK_DMA2>;
 			clock-names = "apb_pclk";
+			arm,pl330-broken-no-flushp;
 		};
 	};
 
@@ -398,6 +401,7 @@
 		#size-cells = <0>;
 		dmas = <&dmac2 10>, <&dmac2 11>;
 		dma-names = "tx", "rx";
+		rockchip,spi-broken-burst-len;
 		status = "disabled";
 	};
 
@@ -411,6 +415,7 @@
 		#size-cells = <0>;
 		dmas = <&dmac2 12>, <&dmac2 13>;
 		dma-names = "tx", "rx";
+		rockchip,spi-broken-burst-len;
 		status = "disabled";
 	};
 };
-- 
2.3.7

[PATCH v3 8/9] spi: rockchip: modify DMA max burst to 1

[Shawn Lin]

From: Addy Ke <addy.ke@rock-chips.com>

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 <addy.ke@rock-chips.com>
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 <olof@lixom.net>
cc: Doug Anderson <dianders@chromium.org>
cc: Sonny Rao <sonnyrao@chromium.org>
---

Changes in v3: None
Changes in v2: None
Changes in v1: None

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

diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 68e7efe..3748ca5 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -199,6 +199,8 @@ struct rockchip_spi {
 	struct sg_table rx_sg;
 	struct rockchip_spi_dma_data dma_rx;
 	struct rockchip_spi_dma_data dma_tx;
+	int quirk;
+#define ROCKCHIP_SPI_BORKEN_BURST_LEN (1<<0) /* broken burst len*/
 };
 
 static inline void spi_enable_chip(struct rockchip_spi *rs, int enable)
@@ -449,7 +451,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->quirk & ROCKCHIP_SPI_BORKEN_BURST_LEN)
+			rxconf.src_maxburst = 1;
+		else
+			rxconf.src_maxburst = rs->n_bytes;
 		dmaengine_slave_config(rs->dma_rx.ch, &rxconf);
 
 		rxdesc = dmaengine_prep_slave_sg(
@@ -466,7 +471,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->quirk & ROCKCHIP_SPI_BORKEN_BURST_LEN)
+			txconf.dst_maxburst = 1;
+		else
+			txconf.dst_maxburst = rs->n_bytes;
 		dmaengine_slave_config(rs->dma_tx.ch, &txconf);
 
 		txdesc = dmaengine_prep_slave_sg(
@@ -699,6 +707,10 @@ static int rockchip_spi_probe(struct platform_device *pdev)
 		goto err_get_fifo_len;
 	}
 
+	if (of_property_read_bool(pdev->dev.of_node,
+				  "rockchip,spi-broken-burst-len"))
+		rs->quirk |= ROCKCHIP_SPI_BORKEN_BURST_LEN;
+
 	spin_lock_init(&rs->lock);
 
 	pm_runtime_set_active(&pdev->dev);
-- 
2.3.7

Re: [PATCH v3 8/9] spi: rockchip: modify DMA max burst to 1

[Mark Brown]

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

On Tue, Sep 01, 2015 at 05:58:44PM +0800, Shawn Lin wrote:

> 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.

If this is a quirk of the DMA controller why is it being fixed with a
property specific to the SPI controller?  Can't the DMA controller just
override the burst size?

> +#define ROCKCHIP_SPI_BORKEN_BURST_LEN (1<<0) /* broken burst len*/

This is misspelt.

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

Re: [PATCH v3 8/9] spi: rockchip: modify DMA max burst to 1

[Shawn Lin]

On 2015/9/3 20:54, Mark Brown wrote:
> On Tue, Sep 01, 2015 at 05:58:44PM +0800, Shawn Lin wrote:
>
>> 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.
>
> If this is a quirk of the DMA controller why is it being fixed with a
> property specific to the SPI controller?  Can't the DMA controller just
> override the burst size?
>

Cool...that's a good question. Actually DMA controller can't do that 
since block peripheral controller(e.g sd/emmc, they always access data 
by block-512Bytes.) can't meet the case. So It's hard for dma controller 
know who is the caller, and what should be done.

The only thing dma controller can do is avoid to execute DMAFLUSHP on 
its own. But how about peripheral controller trigger non-multiple 
access? So that's what I mentioned on covery letter that we should do 
"broken things" for both dma controller and some peripherals.

>> +#define ROCKCHIP_SPI_BORKEN_BURST_LEN (1<<0) /* broken burst len*/
>
> This is misspelt.

Woops, sorry for the bit. Thanks, Mark.

>


-- 
Best Regards
Shawn Lin

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

Re: [PATCH v3 8/9] spi: rockchip: modify DMA max burst to 1

[Mark Brown]

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

On Thu, Sep 03, 2015 at 09:41:43PM +0800, Shawn Lin wrote:
> On 2015/9/3 20:54, Mark Brown wrote:

> >If this is a quirk of the DMA controller why is it being fixed with a
> >property specific to the SPI controller?  Can't the DMA controller just
> >override the burst size?

> Cool...that's a good question. Actually DMA controller can't do that since
> block peripheral controller(e.g sd/emmc, they always access data by
> block-512Bytes.) can't meet the case. So It's hard for dma controller know
> who is the caller, and what should be done.

You could take a guess based on request size, but that's going to be a
bit error prone I imagine.

> The only thing dma controller can do is avoid to execute DMAFLUSHP on its
> own. But how about peripheral controller trigger non-multiple access? So
> that's what I mentioned on covery letter that we should do "broken things"
> for both dma controller and some peripherals.

The other thing it could do is provide the information out via some
internal Linux API which the drivers could query instead of directly
having a DT property in each client driver.  That way at least the DT
only needs the quirk in one place even if the clients need to handle it
individually.

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

Re: [PATCH v3 8/9] spi: rockchip: modify DMA max burst to 1

[Shawn Lin]

On 2015/9/4 1:45, Mark Brown wrote:
> On Thu, Sep 03, 2015 at 09:41:43PM +0800, Shawn Lin wrote:
>> On 2015/9/3 20:54, Mark Brown wrote:
>
>>> If this is a quirk of the DMA controller why is it being fixed with a
>>> property specific to the SPI controller?  Can't the DMA controller just
>>> override the burst size?
>
>> Cool...that's a good question. Actually DMA controller can't do that since
>> block peripheral controller(e.g sd/emmc, they always access data by
>> block-512Bytes.) can't meet the case. So It's hard for dma controller know
>> who is the caller, and what should be done.
>
> You could take a guess based on request size, but that's going to be a
> bit error prone I imagine.
>

yes, not always cover all cases.

>> The only thing dma controller can do is avoid to execute DMAFLUSHP on its
>> own. But how about peripheral controller trigger non-multiple access? So
>> that's what I mentioned on covery letter that we should do "broken things"
>> for both dma controller and some peripherals.
>
> The other thing it could do is provide the information out via some
> internal Linux API which the drivers could query instead of directly
> having a DT property in each client driver.  That way at least the DT
> only needs the quirk in one place even if the clients need to handle it
> individually.

Good idea. Thanks, Mark.

>


-- 
Best Regards
Shawn Lin