[PATCH v3 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: appana.durga.rao@xilinx.com (Appana Durga Kedareswara Rao)
Date: 2017-01-04 13:31:35
Also in:
linux-devicetree, lkml
Hi Thanks for the review...
Hi Kedar, On 04-01-2017 06:54, Kedareswara rao Appana wrote:quoted
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 <redacted>Looks fine. I have a couple of minor comments, if you address them you can add "Reviewed-by: Jose Abreu [off-list ref]" in next version.
Thanks... Sure will fix the comments and will add your' s Reviewed-by.... [snip]
quoted
+ /* + * 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&"ed
+ chan->desc_pendingcount < chan->num_frms) { + dev_dbg(chan->dev, "Frame Store Configuration is not enabledat the");quoted
+ dev_dbg(chan->dev, " H/w level enable Debug info 13 at theh/w level");quoted
+ dev_dbg(chan->dev, " OR Submit the frames upto h/wCapable\n\r");quoted
+ + 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