Thread (6 messages) 6 messages, 2 authors, 2017-01-04

[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
&&
quoted
+	    chan->desc_pendingcount < chan->num_frms) {
+		dev_dbg(chan->dev, "Frame Store Configuration is not enabled
at the");
quoted
+		dev_dbg(chan->dev, " H/w level enable Debug info 13 at the
h/w level");
quoted
+		dev_dbg(chan->dev, " OR Submit the frames upto h/w
Capable\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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help