2010-12-01 10:50:25

by Tomoya MORINAGA

[permalink] [raw]
Subject: [PATCH] dma : EG20T PCH: Fix miss-setting DMA descriptor

From: Tomoya MORINAGA <[email protected]>



Currently, in case of using scatter/gather mode, head of data is not sent to

destination. The cause is second descriptor address is set to NEXT.

The NEXT must have head of descriptor address.

This patch sets head of descriptor address to the NEXT.



Signed-off-by: Tomoya MORINAGA <[email protected]>
---
drivers/dma/pch_dma.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
index 92b6790..a3d54c0 100644
--- a/drivers/dma/pch_dma.c
+++ b/drivers/dma/pch_dma.c
@@ -259,11 +259,6 @@ static void pdc_dostart(struct pch_dma_chan *pd_chan, struct pch_dma_desc* desc)
return;
}

- channel_writel(pd_chan, DEV_ADDR, desc->regs.dev_addr);
- channel_writel(pd_chan, MEM_ADDR, desc->regs.mem_addr);
- channel_writel(pd_chan, SIZE, desc->regs.size);
- channel_writel(pd_chan, NEXT, desc->regs.next);
-
dev_dbg(chan2dev(&pd_chan->chan), "chan %d -> dev_addr: %x\n",
pd_chan->chan.chan_id, desc->regs.dev_addr);
dev_dbg(chan2dev(&pd_chan->chan), "chan %d -> mem_addr: %x\n",
@@ -273,10 +268,16 @@ static void pdc_dostart(struct pch_dma_chan *pd_chan, struct pch_dma_desc* desc)
dev_dbg(chan2dev(&pd_chan->chan), "chan %d -> next: %x\n",
pd_chan->chan.chan_id, desc->regs.next);

- if (list_empty(&desc->tx_list))
+ if (list_empty(&desc->tx_list)) {
+ channel_writel(pd_chan, DEV_ADDR, desc->regs.dev_addr);
+ channel_writel(pd_chan, MEM_ADDR, desc->regs.mem_addr);
+ channel_writel(pd_chan, SIZE, desc->regs.size);
+ channel_writel(pd_chan, NEXT, desc->regs.next);
pdc_set_mode(&pd_chan->chan, DMA_CTL0_ONESHOT);
- else
+ } else {
+ channel_writel(pd_chan, NEXT, virt_to_phys(&desc->regs));
pdc_set_mode(&pd_chan->chan, DMA_CTL0_SG);
+ }

val = dma_readl(pd, CTL2);
val |= 1 << (DMA_CTL2_START_SHIFT_BITS + pd_chan->chan.chan_id);
--
1.6.0.6


2010-12-04 01:33:48

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] dma : EG20T PCH: Fix miss-setting DMA descriptor

2010/12/1 Tomoya MORINAGA <[email protected]>:
> From: Tomoya MORINAGA <[email protected]>
>
>
>
> Currently, in case of using scatter/gather mode, head of data is not sent to
>
> destination. The cause is second descriptor address is set to NEXT.
>
> The NEXT must have head of descriptor address.
>
> This patch sets head of descriptor address to the NEXT.
>
>
>
> Signed-off-by: Tomoya MORINAGA <[email protected]>
> ---
> ?drivers/dma/pch_dma.c | ? 15 ++++++++-------
> ?1 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
> index 92b6790..a3d54c0 100644
> --- a/drivers/dma/pch_dma.c
> +++ b/drivers/dma/pch_dma.c
[..]
> @@ -273,10 +268,16 @@ static void pdc_dostart(struct pch_dma_chan *pd_chan, struct pch_dma_desc* desc)
> ? ? ? ?dev_dbg(chan2dev(&pd_chan->chan), "chan %d -> next: %x\n",
> ? ? ? ? ? ? ? ?pd_chan->chan.chan_id, desc->regs.next);
>
> - ? ? ? if (list_empty(&desc->tx_list))
> + ? ? ? if (list_empty(&desc->tx_list)) {
> + ? ? ? ? ? ? ? channel_writel(pd_chan, DEV_ADDR, desc->regs.dev_addr);
> + ? ? ? ? ? ? ? channel_writel(pd_chan, MEM_ADDR, desc->regs.mem_addr);
> + ? ? ? ? ? ? ? channel_writel(pd_chan, SIZE, desc->regs.size);
> + ? ? ? ? ? ? ? channel_writel(pd_chan, NEXT, desc->regs.next);
> ? ? ? ? ? ? ? ?pdc_set_mode(&pd_chan->chan, DMA_CTL0_ONESHOT);
> - ? ? ? else
> + ? ? ? } else {
> + ? ? ? ? ? ? ? channel_writel(pd_chan, NEXT, virt_to_phys(&desc->regs));

There is a reason for the following comment for virt_to_phys():

/* This function does not give bus mappings for DMA transfers. In
* almost all conceivable cases a device driver should not be using
* this function
*/

...in this case this wants to be:

channel_write(pd_chan, NEXT, desc->txd.phys);

I'll let Yong comment on the correctness of the rest.

--
Dan

2010-12-06 12:58:45

by tip-bot for Yong Wang

[permalink] [raw]
Subject: RE: [PATCH] dma : EG20T PCH: Fix miss-setting DMA descriptor

> > diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
> > index 92b6790..a3d54c0 100644
> > --- a/drivers/dma/pch_dma.c
> > +++ b/drivers/dma/pch_dma.c
> [..]
> > @@ -273,10 +268,16 @@ static void pdc_dostart(struct pch_dma_chan
> *pd_chan, struct pch_dma_desc* desc)
> > ? ? ? ?dev_dbg(chan2dev(&pd_chan->chan), "chan %d -> next: %x\n",
> > ? ? ? ? ? ? ? ?pd_chan->chan.chan_id, desc->regs.next);
> >
> > - ? ? ? if (list_empty(&desc->tx_list))
> > + ? ? ? if (list_empty(&desc->tx_list)) {
> > + ? ? ? ? ? ? ? channel_writel(pd_chan, DEV_ADDR,
> desc->regs.dev_addr);
> > + ? ? ? ? ? ? ? channel_writel(pd_chan, MEM_ADDR,
> desc->regs.mem_addr);
> > + ? ? ? ? ? ? ? channel_writel(pd_chan, SIZE, desc->regs.size);
> > + ? ? ? ? ? ? ? channel_writel(pd_chan, NEXT, desc->regs.next);
> > ? ? ? ? ? ? ? ?pdc_set_mode(&pd_chan->chan,
> DMA_CTL0_ONESHOT);
> > - ? ? ? else
> > + ? ? ? } else {
> > + ? ? ? ? ? ? ? channel_writel(pd_chan, NEXT,
> virt_to_phys(&desc->regs));
>
> There is a reason for the following comment for virt_to_phys():
>
> /* This function does not give bus mappings for DMA transfers. In
> * almost all conceivable cases a device driver should not be using
> * this function
> */
>
> ...in this case this wants to be:
>
> channel_write(pd_chan, NEXT, desc->txd.phys);
>
> I'll let Yong comment on the correctness of the rest.
>

Hi Dan,

Apart from the issue you pointed out, the patch looks good to me. If it is convenient for you, please help make the change and merge this patch. Otherwise, please let Tomoya know that you want him to send an updated patch. Thanks!

Acked-by: Yong Wang <[email protected]>

-Yong