Re: [PATCH 2/2] dmaengine: usb-dmac: Add Renesas USB DMA Controller (USB-DMAC) driver

On Thu, Mar 05, 2015 at 12:35:49PM +0000, yoshihiro shimoda wrote:

Hi Vinod,

Thank you for your review!

quoted

On Mon, Feb 09, 2015 at 05:14:05PM +0900, Yoshihiro Shimoda wrote:

quoted

+struct usb_dmac_chan {
+	struct dma_chan chan;
+	void __iomem *iomem;
+	unsigned int index;
+
+	spinlock_t lock;
+
+	struct {
+		struct list_head free;
+		struct list_head pending;
+		struct list_head active;
+		struct list_head done;
+		struct list_head wait;
+		struct usb_dmac_desc *running;
+		struct usb_dmac_desc *last_done;
+
+		struct list_head chunks_free;
+
+		struct list_head pages;

Thats too many lists, do we need so many? Shouldn't free and done be same
thing. Similarly whats meant by wait here?
Do you submit multiple descriptors to HW?

No, I don't submit multiple descriptors to HW.
So, as you say in the end of this email, I am thinking that I should use virt-dma infrastructure.

That will be good and greatly simplify..

quoted

quoted

+static enum dma_status usb_dmac_tx_status(struct dma_chan *chan,
+					  dma_cookie_t cookie,
+					  struct dma_tx_state *txstate)
+{
+	struct usb_dmac_chan *uchan = to_usb_dmac_chan(chan);
+	enum dma_status status;
+	unsigned long flags;
+	unsigned int residue;
+
+	status = dma_cookie_status(chan, cookie, txstate);
+	/* a client driver will get residue after DMA_COMPLETE */
+	if (!txstate)
+		return status;
+
+	spin_lock_irqsave(&uchan->lock, flags);
+	if (status == DMA_COMPLETE)
+		residue = usb_dmac_chan_get_residue_if_complete(uchan);

if it is completed then residue should be zero, so why are we computing this

This USB-DMAC has a function to detect a USB specific packet (called short-length-packet).
If the USB-DMAC detects it, the USB-DMAC assumes the USB-DMAC completes the transfer.

For example:
 - A client driver submits 2048 bytes as RX.
 - When a USB controller received 512 + 488 bytes totally,
   the USB-DMAC detected it and completed the transfer.
  - However, a USB controller just knows the bytes of last packet (In this case, 488byte.)
        So, the USB controller driver cannot know that it got how many bytes.
    Therefore, this USB-DMAC driver is computing this bytes.

I'm not sure about the detail, but cppi41.c seems to compute the residue even if it is completed.

I suspected this to be case, so you are confirming my hunch, thanks :)

-- 
~Vinod

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html