lkml.org 
[lkml]   [2017]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v3 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
Date
Hi 

Thanks for the review...
>
> Hi Kedar,
>
>
> On 04-01-2017 06:54, Kedareswara rao Appana wrote:
> > When VDMA is configured for more than one frame in the h/w for example
> > h/w is configured for n number of frames and user Submits n number of
> > frames and triggered the DMA using issue_pending API.
> > In the current driver flow we are submitting one frame at a time but
> > we should submit all the n number of frames at one time as the h/w Is
> > configured for n number of frames.
> >
> > This patch fixes this issue.
> >
> > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
>
> Looks fine. I have a couple of minor comments, if you address them you can add
> "Reviewed-by: Jose Abreu <joabreu@synopsys.com>"
> in next version.

Thanks...
Sure will fix the comments and will add your' s Reviewed-by....

[snip]
> > + /*
> > + * Note: When VDMA is built with default h/w configuration
> > + * On the S2MM(recv) side user should submit frames upto
> > + * H/W configured. If users submits less than h/w configured
> > + * VDMA engine tries to write to a invalid location
> > + * Results undefined behaviour/memory corruption.
> > + *
> > + * If user would like to submit frames less than h/w capable
> > + * On S2MM side please enable debug info 13 at the h/w level
> > + * It will allows the frame buffers numbers to be modified at runtime.
> > + */
> > + if (!chan->has_fstoreconfig && chan->direction == DMA_DEV_TO_MEM
> &&
> > + chan->desc_pendingcount < chan->num_frms) {
> > + dev_dbg(chan->dev, "Frame Store Configuration is not enabled
> at the");
> > + dev_dbg(chan->dev, " H/w level enable Debug info 13 at the
> h/w level");
> > + dev_dbg(chan->dev, " OR Submit the frames upto h/w
> Capable\n\r");
> > +
> > + return;
> > + }
>
> Hmm, may dev_warn would be more suitable because with dev_dbg and no
> dynamic debug enabled user will not know what happened. Also, I am aware
> that in direction DMA_MEM_TO_DEV there will be no corruption in PC side but it
> will be corruption in VDMA side because it will read from invalid memory
> locations. Maybe drop the check for channel direction.

Sure will fix it in the next version....

>
> I am also not fancy about dropping prints that are not grep'able (you do not
> break line in each print so a user searching for the whole string will not find it).
> Try to do a line break in each print or change the string to be smaller.
>

Sure will add a line break in each print in the next version...

Regards,
Kedar.

> Best regards,
> Jose Miguel Abreu
>

\
 
 \ /
  Last update: 2017-01-04 14:32    [W:0.113 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site