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

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

Post by Dan Willia » Sun, 05 Dec 2010 10:40:01


2010/12/1 Tomoya MORINAGA < XXXX@XXXXX.COM >:
> rivers/dma/pch_dma.c | 15 ++++++++------- >> 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)> > ev_dbg(chan2dev(&pd_>han->chan), "chan>%d -> next: %x\>",
> gt;d_chan->chan.ch>n_id, desc->r>gs>next);
>
> - if (li>t_empty(&de>c->tx_list))
> + if >list_empty(&d>sc->tx_list)) {
> + channel_writel>pd_chan, DEV_ADDR> desc->regs.dev_addr);
> + channel>writel(pd_chan, M>M_ADDR, desc->regs.mem_addr);
> + > channel_wr>tel(pd_chan, SIZE, desc->regs.size);
> + gt;cha>nel_writel(pd_chan, NEXT, desc->regs.next);
> gt;dc_set_m>de(&pd_chan->cha>, 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
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

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

Post by Dan Willia » Sun, 05 Dec 2010 10:40:01

2010/12/1 Tomoya MORINAGA < XXXX@XXXXX.COM >:
> rivers/dma/pch_dma.c | 15 ++++++++------- >> 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)> > ev_dbg(chan2dev(&pd_>han->chan), "chan>%d -> next: %x\>",
> gt;d_chan->chan.ch>n_id, desc->r>gs>next);
>
> - if (li>t_empty(&de>c->tx_list))
> + if >list_empty(&d>sc->tx_list)) {
> + channel_writel>pd_chan, DEV_ADDR> desc->regs.dev_addr);
> + channel>writel(pd_chan, M>M_ADDR, desc->regs.mem_addr);
> + > channel_wr>tel(pd_chan, SIZE, desc->regs.size);
> + gt;cha>nel_writel(pd_chan, NEXT, desc->regs.next);
> gt;dc_set_m>de(&pd_chan->cha>, 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
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/

 
 
 

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

Post by Dan Willia » Sun, 05 Dec 2010 10:40:01

2010/12/1 Tomoya MORINAGA < XXXX@XXXXX.COM >:
> rivers/dma/pch_dma.c | 15 ++++++++------- >> 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)> > ev_dbg(chan2dev(&pd_>han->chan), "chan>%d -> next: %x\>",
> gt;d_chan->chan.ch>n_id, desc->r>gs>next);
>
> - if (li>t_empty(&de>c->tx_list))
> + if >list_empty(&d>sc->tx_list)) {
> + channel_writel>pd_chan, DEV_ADDR> desc->regs.dev_addr);
> + channel>writel(pd_chan, M>M_ADDR, desc->regs.mem_addr);
> + > channel_wr>tel(pd_chan, SIZE, desc->regs.size);
> + gt;cha>nel_writel(pd_chan, NEXT, desc->regs.next);
> gt;dc_set_m>de(&pd_chan->cha>, 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
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

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

Post by Dan Willia » Sun, 05 Dec 2010 10:40:01

2010/12/1 Tomoya MORINAGA < XXXX@XXXXX.COM >:
> rivers/dma/pch_dma.c | 15 ++++++++------- >> 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)> > ev_dbg(chan2dev(&pd_>han->chan), "chan>%d -> next: %x\>",
> gt;d_chan->chan.ch>n_id, desc->r>gs>next);
>
> - if (li>t_empty(&de>c->tx_list))
> + if >list_empty(&d>sc->tx_list)) {
> + channel_writel>pd_chan, DEV_ADDR> desc->regs.dev_addr);
> + channel>writel(pd_chan, M>M_ADDR, desc->regs.mem_addr);
> + > channel_wr>tel(pd_chan, SIZE, desc->regs.size);
> + gt;cha>nel_writel(pd_chan, NEXT, desc->regs.next);
> gt;dc_set_m>de(&pd_chan->cha>, 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
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

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

Post by Wang, Yong » Tue, 07 Dec 2010 22:00:03

> > diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
> >
> > - if (list>empty(&desc>>>x_list))
> > + if (l>st_empty(&des>->tx_list)) {
> > + channel_writ>l(pd_c>an, DEV_ADDR,
> >e>c->regs.dev_addr);
> > + chann>l_writ>l(pd_chan, MEM_AD>R>
> desc->regs.mem_addr);
> > + > channel_writ>l>pd_chan, SIZE, desc->regs.size);
> > + gt;chann>l>writel(pd_chan, NEXT, desc->reg>.next);> > > >c>set_mode(&pd>c>an->chan,
> DMA>C>L0_ONESHOT);
> > - else
> > + gt;} else {
> > + > > ch>nnel_writel(pd_chan, NEXT,
> virt_to_phys(&desc->regs));
>
> T>ere>is a reason for the following comment for virt_to_phys():
>
> /* >his function does not give bus mappings for DMA transfers. In
> * al>ost all conceivable case> a dev>ce >river should not be using
> * >thi> function
> */
>
> ...in this case >his 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 upda>ed patch. Thanks!

Acked-by: Yong Wang < XXXX@XXXXX.COM >

-Yong

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/