2015-08-27 08:49:19

by Shawn Lin

[permalink] [raw]
Subject: [PATCH 0/5] Fix broken DMAFLUSHP on Rockchips platform


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.



Shawn Lin (5):
DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit
DMA: pl330: add quirk for broken no flushp
ARM: dts: Add broken-no-flushp quirk for rk3288 platform
ARM: dts: Add broken-no-flushp quirk for rk3xxx platform
Documentation: arm-pl330: add description of broken-no-flushp

.../devicetree/bindings/dma/arm-pl330.txt | 1 +
arch/arm/boot/dts/rk3288.dtsi | 3 +
arch/arm/boot/dts/rk3xxx.dtsi | 3 +
drivers/dma/pl330.c | 101 +++++++++++++++------
4 files changed, 79 insertions(+), 29 deletions(-)

--
2.3.7


2015-08-27 08:49:38

by Shawn Lin

[permalink] [raw]
Subject: [PATCH 1/5] DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit

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

Signed-off-by: Boojin Kim <[email protected]>
Signed-off-by: Addy Ke <[email protected]>
Signed-off-by: Shawn Lin <[email protected]>
cc: Heiko Stuebner <[email protected]>
cc: Doug Anderson <[email protected]>
cc: Olof Johansson <[email protected]>
Reviewed-and-tested-by: Sonny Rao <[email protected]>

---

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

2015-08-27 08:49:49

by Shawn Lin

[permalink] [raw]
Subject: [PATCH 2/5] DMA: pl330: add quirk for broken no flushp

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

Signed-off-by: Addy Ke <[email protected]>
Signed-off-by: Shawn Lin <[email protected]>
cc: Heiko Stuebner <[email protected]>
cc: Doug Anderson <[email protected]>
cc: Olof Johansson <[email protected]>
Reviewed-and-tested-by: Sonny Rao <[email protected]>
---

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..c300dc2 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 = "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

2015-08-27 08:50:13

by Shawn Lin

[permalink] [raw]
Subject: [PATCH 3/5] ARM: dts: Add broken-no-flushp quirk for rk3288 platform

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

Signed-off-by: Addy Ke <[email protected]>
Signed-off-by: Shawn Lin <[email protected]>
cc: Heiko Stuebner <[email protected]>
cc: Doug Anderson <[email protected]>
cc: Olof Johansson <[email protected]>
Reviewed-and-tested-by: Sonny Rao <[email protected]>
---

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 22316d0..c7ec98a 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";
+ broken-no-flushp;
};

dmac_bus_ns: dma-controller@ff600000 {
@@ -155,6 +156,7 @@
clocks = <&cru ACLK_DMAC1>;
clock-names = "apb_pclk";
status = "disabled";
+ broken-no-flushp;
};

dmac_bus_s: dma-controller@ffb20000 {
@@ -165,6 +167,7 @@
#dma-cells = <1>;
clocks = <&cru ACLK_DMAC1>;
clock-names = "apb_pclk";
+ broken-no-flushp;
};
};

--
2.3.7

2015-08-27 08:50:12

by Shawn Lin

[permalink] [raw]
Subject: [PATCH 4/5] ARM: dts: Add broken-no-flushp quirk for rk3xxx platform

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

Signed-off-by: Shawn Lin <[email protected]>
cc: Heiko Stuebner <[email protected]>
cc: Doug Anderson <[email protected]>
cc: Olof Johansson <[email protected]>
---

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 a2ae9f3..8bf9c71 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";
+ broken-no-flushp;
};

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

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

--
2.3.7

2015-08-27 08:50:26

by Shawn Lin

[permalink] [raw]
Subject: [PATCH 5/5] Documentation: arm-pl330: add description of broken-no-flushp

Signed-off-by: Shawn Lin <[email protected]>
---

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..0f19268 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
+ - broken-no-flushp: quirk for avoiding to execute DMAFLUSHP

Example:

--
2.3.7

2015-08-27 08:57:32

by Heiko Stübner

[permalink] [raw]
Subject: Re: [PATCH 1/5] DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit

Hi Shawn,


you're loosing the original author. Your can override the patch-author (away
from the mail sender) via a "From:" line at the top of the commit message,
like:

------
From: Boojin Kim <[email protected]>

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


> This patch adds to support burst mode for dev-to-mem and
> mem-to-dev transmit.
>
> Signed-off-by: Boojin Kim <[email protected]>
> Signed-off-by: Addy Ke <[email protected]>
> Signed-off-by: Shawn Lin <[email protected]>
> cc: Heiko Stuebner <[email protected]>
> cc: Doug Anderson <[email protected]>
> cc: Olof Johansson <[email protected]>
> Reviewed-and-tested-by: Sonny Rao <[email protected]>
>
> ---
>
> 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);
> }

2015-08-27 11:05:15

by Shawn Lin

[permalink] [raw]
Subject: Re: [PATCH 1/5] DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit

On 2015/8/27 16:57, Heiko Stuebner wrote:
> Hi Shawn,
>
>
> you're loosing the original author. Your can override the patch-author (away
> from the mail sender) via a "From:" line at the top of the commit message,
> like:
>

Woops.... sorry for missing this bit. Thanks for pointing out that. :)

Patch 1 is from Boojin, and patch 2,3 attribute to Addy(Actually I
rebase it for linux-next from v3.14). patch 4 and 5 comes from myself.

It will be fixed from the next ones if any.

> ------
> From: Boojin Kim <[email protected]>
>
> This patch adds to support burst mode for dev-to-mem and
> mem-to-dev transmit.
> ...
> ------
>
>
>> This patch adds to support burst mode for dev-to-mem and
>> mem-to-dev transmit.
>>
>> Signed-off-by: Boojin Kim <[email protected]>
>> Signed-off-by: Addy Ke <[email protected]>
>> Signed-off-by: Shawn Lin <[email protected]>
>> cc: Heiko Stuebner <[email protected]>
>> cc: Doug Anderson <[email protected]>
>> cc: Olof Johansson <[email protected]>
>> Reviewed-and-tested-by: Sonny Rao <[email protected]>
>>
>> ---
>>
>> 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);
>> }
>
>
>
>


--
Best Regards
Shawn Lin

2015-08-27 12:57:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/5] DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit

2015-08-27 17:48 GMT+09:00 Shawn Lin <[email protected]>:
>
> This patch adds to support burst mode for dev-to-mem and
> mem-to-dev transmit.
>
> Signed-off-by: Boojin Kim <[email protected]>
> Signed-off-by: Addy Ke <[email protected]>
> Signed-off-by: Shawn Lin <[email protected]>
> cc: Heiko Stuebner <[email protected]>
> cc: Doug Anderson <[email protected]>
> cc: Olof Johansson <[email protected]>
> Reviewed-and-tested-by: Sonny Rao <[email protected]>

For the entire patchset: I would prefer to see someone's
reviewed/tested tag in his response. Sending a version 1 of patchset
(regardless of Boojin Kim's work two years ago) with such tag could
mean anything. I cannot verify it easily (unless digging somewhere...
or asking people). You could add for example: Reviewed-by Santa Claus.
Should I sent a letter to him asking for confirmation? :)

More seriously - reviewed-by is a statement (please look at
Documentation/SubmittingPatches) and you cannot force someone to make
that statement. He must make such statement on his own.

That's all from my side since I don't feel skilled enough to review the code.

Best regards,
Krzysztof

>
> ---
>
> 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 dmaengine" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-08-27 13:00:01

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 5/5] Documentation: arm-pl330: add description of broken-no-flushp

2015-08-27 17:49 GMT+09:00 Shawn Lin <[email protected]>:
> Signed-off-by: Shawn Lin <[email protected]>
> ---
>
> 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..0f19268 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
> + - broken-no-flushp: quirk for avoiding to execute DMAFLUSHP

It looks like a device or vendor specific quirk so the property should
have a prefix. For example:
arm,pl330-broken-no-flushp

Best regards,
Krzysztof

2015-08-27 13:18:07

by Shawn Lin

[permalink] [raw]
Subject: Re: [PATCH 5/5] Documentation: arm-pl330: add description of broken-no-flushp

On 2015/8/27 20:59, Krzysztof Kozlowski write:
> 2015-08-27 17:49 GMT+09:00 Shawn Lin <[email protected]>:
>> Signed-off-by: Shawn Lin <[email protected]>
>> ---
>>
>> 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..0f19268 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
>> + - broken-no-flushp: quirk for avoiding to execute DMAFLUSHP
>
> It looks like a device or vendor specific quirk so the property should
> have a prefix. For example:
> arm,pl330-broken-no-flushp
>

Got it.
Thanks, Krzysztof.

> Best regards,
> Krzysztof
>
>
>


--
Best Regards
Shawn Lin

2015-08-27 13:28:31

by Shawn Lin

[permalink] [raw]
Subject: Re: [PATCH 1/5] DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit

在 2015/8/27 20:57, Krzysztof Kozlowski 写道:
> 2015-08-27 17:48 GMT+09:00 Shawn Lin <[email protected]>:
>>
>> This patch adds to support burst mode for dev-to-mem and
>> mem-to-dev transmit.
>>
>> Signed-off-by: Boojin Kim <[email protected]>
>> Signed-off-by: Addy Ke <[email protected]>
>> Signed-off-by: Shawn Lin <[email protected]>
>> cc: Heiko Stuebner <[email protected]>
>> cc: Doug Anderson <[email protected]>
>> cc: Olof Johansson <[email protected]>
>> Reviewed-and-tested-by: Sonny Rao <[email protected]>
>
> For the entire patchset: I would prefer to see someone's
> reviewed/tested tag in his response. Sending a version 1 of patchset
> (regardless of Boojin Kim's work two years ago) with such tag could
> mean anything. I cannot verify it easily (unless digging somewhere...
> or asking people). You could add for example: Reviewed-by Santa Claus.
> Should I sent a letter to him asking for confirmation? :)
>

:) yes, you are right. I should comply with the rule, even if the
patchest had been reviewed or tested by someone on another tree.

> More seriously - reviewed-by is a statement (please look at
> Documentation/SubmittingPatches) and you cannot force someone to make
> that statement. He must make such statement on his own.
>
> That's all from my side since I don't feel skilled enough to review the code.
>
> Best regards,
> Krzysztof
>
>>
>> ---
>>
>> 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 dmaengine" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>


--
Best Regards
Shawn Lin

2015-08-28 18:25:24

by Sonny Rao

[permalink] [raw]
Subject: Re: [PATCH 1/5] DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit

On Thu, Aug 27, 2015 at 6:28 AM, Shawn Lin <[email protected]> wrote:
> 在 2015/8/27 20:57, Krzysztof Kozlowski 写道:
>>
>> 2015-08-27 17:48 GMT+09:00 Shawn Lin <[email protected]>:
>>>
>>>
>>> This patch adds to support burst mode for dev-to-mem and
>>> mem-to-dev transmit.
>>>
>>> Signed-off-by: Boojin Kim <[email protected]>
>>> Signed-off-by: Addy Ke <[email protected]>
>>> Signed-off-by: Shawn Lin <[email protected]>
>>> cc: Heiko Stuebner <[email protected]>
>>> cc: Doug Anderson <[email protected]>
>>> cc: Olof Johansson <[email protected]>
>>> Reviewed-and-tested-by: Sonny Rao <[email protected]>
>>
>>
>> For the entire patchset: I would prefer to see someone's
>> reviewed/tested tag in his response. Sending a version 1 of patchset
>> (regardless of Boojin Kim's work two years ago) with such tag could
>> mean anything. I cannot verify it easily (unless digging somewhere...
>> or asking people). You could add for example: Reviewed-by Santa Claus.
>> Should I sent a letter to him asking for confirmation? :)
>>
>
> :) yes, you are right. I should comply with the rule, even if the patchest
> had been reviewed or tested by someone on another tree.

Hi, yeah I reviewed on a different tree, so you shouldn't put that tag
here, thanks for removing it.
I can re-review if you'd like.

>
>
>> More seriously - reviewed-by is a statement (please look at
>> Documentation/SubmittingPatches) and you cannot force someone to make
>> that statement. He must make such statement on his own.
>>
>> That's all from my side since I don't feel skilled enough to review the
>> code.
>>
>> Best regards,
>> Krzysztof
>>
>>>
>>> ---
>>>
>>> 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 dmaengine" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>>
>
>
> --
> Best Regards
> Shawn Lin
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2015-08-28 23:40:59

by Shawn Lin

[permalink] [raw]
Subject: Re: [PATCH 1/5] DMA: pl330: support burst mode for dev-to-mem and mem-to-dev transmit

在 2015/8/29 2:19, Sonny Rao 写道:
> On Thu, Aug 27, 2015 at 6:28 AM, Shawn Lin <[email protected]> wrote:
>> 在 2015/8/27 20:57, Krzysztof Kozlowski 写道:
>>>
>>> 2015-08-27 17:48 GMT+09:00 Shawn Lin <[email protected]>:
>>>>
>>>>
>>>> This patch adds to support burst mode for dev-to-mem and
>>>> mem-to-dev transmit.
>>>>
>>>> Signed-off-by: Boojin Kim <[email protected]>
>>>> Signed-off-by: Addy Ke <[email protected]>
>>>> Signed-off-by: Shawn Lin <[email protected]>
>>>> cc: Heiko Stuebner <[email protected]>
>>>> cc: Doug Anderson <[email protected]>
>>>> cc: Olof Johansson <[email protected]>
>>>> Reviewed-and-tested-by: Sonny Rao <[email protected]>
>>>
>>>
>>> For the entire patchset: I would prefer to see someone's
>>> reviewed/tested tag in his response. Sending a version 1 of patchset
>>> (regardless of Boojin Kim's work two years ago) with such tag could
>>> mean anything. I cannot verify it easily (unless digging somewhere...
>>> or asking people). You could add for example: Reviewed-by Santa Claus.
>>> Should I sent a letter to him asking for confirmation? :)
>>>
>>
>> :) yes, you are right. I should comply with the rule, even if the patchest
>> had been reviewed or tested by someone on another tree.
>
> Hi, yeah I reviewed on a different tree, so you shouldn't put that tag
> here, thanks for removing it.
> I can re-review if you'd like.
>

Hi, sunny, Thanks for reply.
It's very great if you can help me re-review the patchset here. :)

>>
>>
>>> More seriously - reviewed-by is a statement (please look at
>>> Documentation/SubmittingPatches) and you cannot force someone to make
>>> that statement. He must make such statement on his own.
>>>
>>> That's all from my side since I don't feel skilled enough to review the
>>> code.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>>>
>>>> ---
>>>>
>>>> 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 dmaengine" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>>
>>>
>>>
>>
>>
>> --
>> Best Regards
>> Shawn Lin
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
>
>


--
Best Regards
Shawn Lin