Re: [RFC PATCH v5 1/9] mmc: dw_mmc: Add external dma interface support

[Shawn Lin <shawn.lin@rock-chips.com>]

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

在 2015/8/17 5:10, Doug Anderson 写道:
> Heiko,
>
> On Fri, Aug 14, 2015 at 3:13 PM, Heiko Stübner <heiko@sntech.de> wrote:
>> Hi Shawn,
>>
>> Am Freitag, 14. August 2015, 16:34:35 schrieb Shawn Lin:
>>> DesignWare MMC Controller can supports two types of DMA
>>> mode: external dma and internal dma. We get a RK312x platform
>>> integrated dw_mmc and ARM pl330 dma controller. This patch add
>>> edmac ops to support these platforms. I've tested it on RK312x
>>> platform with edmac mode and RK3288 platform with idmac mode.
>>>
>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> judging by your "from", I guess you're running this on some older Rockchip soc
>> without the idma? Because I tried testing this on a Radxa Rock, but only got
>> failures, from the start (failed to read card status register). In PIO mode
>> everything works again.
>>
>>
>> I guess I overlooked just some tiny detail, but to me the dma channel ids seem
>> correct after all. Maybe you have any hints what I'm doing wrong?
>
> If I were a guessing man (which I'm not), I'd guess that perhaps
> you're running into troubles with our friend the PL330.
>
> There appear to be strange issues with the PL330 on Rockchip SoCs.  I
> was only peripherally involved with them, but I know at least about
> some of the patches in our tree, like:
>

Thanks, Doug, that's the root cause. PL330 on Rockchip Socs need your 
patches, and we might need another patch to limit pl330 burst_len to 
16(Hmm...seems another quirks for rockchip but not on your tree);

Hi Heiko,
I just get a Radxa Rock luckily and test my patch based on 
http://radxa.com/Rock/Linux_Mainline. Yes, it can't work.

If you test my patchset on Rockchip platform, pls apply pl330 patch from 
my tree based on kernel 4.2-RC3. AND, temporary hack dw_mmc to limit 
pl330 burst_len to 16.
This is a dmaengine or pl330 problem(I guest it should be upstreamed 
later?), but my patchset is for *generic* dw_mmc to support emdac, so 
other platforms should never need the hack of pl330.

pl330.patch is for pl330 changes
r3xxx.patch is for pl330 quirks

AND pls limit burst_len to 16 for BROKRN pl330 of rockchips.
static int dw_mci_edmac_start_dma(
...
+        u32 burst_limit = 0;
+        u32 mburst;
+        u32 idx, rx_wmark, tx_wmark;

...
         /* Match burst msize with external dma config */
         fifoth_val = mci_readl(host, FIFOTH);
-	cfg.dst_maxburst = mszs[(fifoth_val >> 28) & 0x7];
-	cfg.src_maxburst = cfg.dst_maxburst;
+	/* HACK for BROKEN pl330 */
+       mburst = mszs[(fifoth_val >> 28) & 0x7];
+       burst_limit = 16;
+       if (mburst > burst_limit) {
+               mburst = burst_limit;
+               idx = (ilog2(mburst) > 0) ? (ilog2(mburst) - 1) : 0;
+               rx_wmark = mszs[idx] - 1;
+               tx_wmark = (host->fifo_depth) / 2;
+               fifoth_val = SDMMC_SET_FIFOTH(idx, rx_wmark, tx_wmark);
+               mci_writel(host, FIFOTH, fifoth_val);
+       }
+	cfg.dst_maxburst = mburst;
+       cfg.src_maxburst = cfg.dst_maxburst;


> https://chromium-review.googlesource.com/237607
> FROMLIST: DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit
>
> https://chromium-review.googlesource.com/237393
> CHROMIUM: dmaengine: pl330: support quirks for some broken
>
> https://chromium-review.googlesource.com/237396
> CHROMIUM: dmaengine: pl330: add quirk for broken no flushp
>
> https://chromium-review.googlesource.com/237394
> CHROMIUM: ARM: dts: rockchip: Add broken-no-flushp into rk3288.dtsi
>
> https://chromium-review.googlesource.com/242063
> CHROMIUM: ASoC: rockchip_i2s: modify DMA max burst to 1
>
>
>


-- 
Shawn Lin

[-- Attachment #2: pl330.patch --]
[-- Type: text/plain, Size: 7954 bytes --]

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index ecab4ea..b2a950c 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -34,6 +34,14 @@
 #define PL330_MAX_IRQS		32
 #define PL330_MAX_PERI		32
 
+#define PL330_QUIRK_BROKEN_NO_FLUSHP BIT(0)
+
+enum pl330_cond {
+	SINGLE,
+	BURST,
+	ALWAYS,
+};
+
 enum pl330_cachectrl {
 	CCTRL0,		/* Noncacheable and nonbufferable */
 	CCTRL1,		/* Bufferable only */
@@ -344,12 +352,6 @@ enum pl330_dst {
 	DST,
 };
 
-enum pl330_cond {
-	SINGLE,
-	BURST,
-	ALWAYS,
-};
-
 struct dma_pl330_desc;
 
 struct _pl330_req {
@@ -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 = "broken-no-flushp",
+		.id = PL330_QUIRK_BROKEN_NO_FLUSHP,
+	}
 };
 
 struct dma_pl330_desc {
@@ -1137,47 +1150,64 @@ static inline int _ldst_memtomem(unsigned dry_run, u8 buf[],
 	return off;
 }
 
-static inline int _ldst_devtomem(unsigned dry_run, u8 buf[],
+static inline int _ldst_devtomem(struct pl330_dmac *pi,unsigned dry_run, u8 buf[],
 		const struct _xfer_spec *pxs, int cyc)
 {
 	int off = 0;
+	enum pl330_cond cond;
+
+	if (pi->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], 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);
+
+		if (!(pi->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[],
+static inline int _ldst_memtodev(struct pl330_dmac *pi, unsigned dry_run, u8 buf[],
 		const struct _xfer_spec *pxs, int cyc)
 {
 	int off = 0;
+	enum pl330_cond cond;
+
+	if (pi->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], 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_FLUSHP(dry_run, &buf[off], pxs->desc->peri);
+		off += _emit_STP(dry_run, &buf[off], cond, pxs->desc->peri);
+
+		if (!(pi->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 *pi,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(pi, dry_run, &buf[off], pxs, cyc);
 		break;
 	case DMA_DEV_TO_MEM:
-		off += _ldst_devtomem(dry_run, &buf[off], pxs, cyc);
+		off += _ldst_devtomem(pi, dry_run, &buf[off], pxs, cyc);
 		break;
 	case DMA_MEM_TO_MEM:
 		off += _ldst_memtomem(dry_run, &buf[off], pxs, cyc);
@@ -1191,7 +1221,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 *pi, unsigned dry_run, u8 buf[],
 		unsigned long *bursts, const struct _xfer_spec *pxs)
 {
 	int cyc, cycmax, szlp, szlpend, szbrst, off;
@@ -1214,7 +1244,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(pi, 1, buf, pxs, 1);
 
 	lpend.cond = ALWAYS;
 	lpend.forever = false;
@@ -1246,7 +1276,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(pi, dry_run, &buf[off], pxs, cyc);
 
 	lpend.cond = ALWAYS;
 	lpend.forever = false;
@@ -1269,7 +1299,7 @@ static inline int _loop(unsigned dry_run, u8 buf[],
 	return off;
 }
 
-static inline int _setup_loops(unsigned dry_run, u8 buf[],
+static inline int _setup_loops(struct pl330_dmac *pi, unsigned dry_run, u8 buf[],
 		const struct _xfer_spec *pxs)
 {
 	struct pl330_xfer *x = &pxs->desc->px;
@@ -1279,14 +1309,14 @@ static inline int _setup_loops(unsigned dry_run, u8 buf[],
 
 	while (bursts) {
 		c = bursts;
-		off += _loop(dry_run, &buf[off], &c, pxs);
+		off += _loop(pi, dry_run, &buf[off], &c, pxs);
 		bursts -= c;
 	}
 
 	return off;
 }
 
-static inline int _setup_xfer(unsigned dry_run, u8 buf[],
+static inline int _setup_xfer(struct pl330_dmac *pi,unsigned dry_run, u8 buf[],
 		const struct _xfer_spec *pxs)
 {
 	struct pl330_xfer *x = &pxs->desc->px;
@@ -1298,7 +1328,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(pi, dry_run, &buf[off], pxs);
 
 	return off;
 }
@@ -1307,7 +1337,7 @@ 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,
+static int _setup_req(struct pl330_dmac *pi,unsigned dry_run, struct pl330_thread *thrd,
 		unsigned index, struct _xfer_spec *pxs)
 {
 	struct _pl330_req *req = &thrd->req[index];
@@ -1325,7 +1355,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(pi, dry_run, &buf[off], pxs);
 
 	/* DMASEV peripheral/event */
 	off += _emit_SEV(dry_run, &buf[off], thrd->ev);
@@ -1419,7 +1449,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;
 
@@ -1433,7 +1463,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;
 
@@ -2557,7 +2587,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 +2732,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);
 	}
@@ -2778,6 +2808,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);
 
@@ -2797,6 +2828,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))

[-- Attachment #3: rk3xxxx.patch --]
[-- Type: text/plain, Size: 426 bytes --]

diff --git a/arch/arm/boot/dts/rk3xxx.dtsi b/arch/arm/boot/dts/rk3xxx.dtsi
index a2ae9f3..fb5b4ec 100644
--- a/arch/arm/boot/dts/rk3xxx.dtsi
+++ b/arch/arm/boot/dts/rk3xxx.dtsi
@@ -94,6 +94,7 @@
 
 		dmac2: dma-controller@20078000 {
 			compatible = "arm,pl330", "arm,primecell";
+			broken-no-flushp;
 			reg = <0x20078000 0x4000>;
 			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;