Hi Marc,

Sorry for the late response.

On 03/02/2016 11:08 AM, Ramesh Shanmugasundaram wrote:

quoted

quoted

quoted

quoted

I see no locking for the tx-path.

I am not sure why locking is needed in tx-path?

If the tx-path is shared between both channels you need locking as
the networking subsystem may send one frame to each controller at the
same time.

Yes. Tx completion interrupt is shared by both channels but the Tx
FIFO, echo skbs, head, tail are maintained on a per channel basis.
Yes, net subsystem can send one frame to each channel at the same time
and the tx completion interrupt handler will traverse each channel &
update per channel context accordingly.

Ok. Which instruction starts the transmit? Please add a comment to the
code.

Please see

---<snip>---

+	/* Start Tx: Write 0xff to CFPC to increment the CPU-side
+	 * pointer for the Common FIFO
+	 */
+	rcar_canfd_write(priv, RCANFD_CFPCTR(ch, RCANFD_CFFIFO_IDX), 0xff);

---<snip>---

You stop the tx_queue after this, so your code is racy, as the
interrupt might happen in between.

It is a very good point. Thank you. 
I managed to reproduce this issue once with 50Kbps rate and ignoring -ENOBUFS in user space. It also exposed another subtle issue in the echo skb management where the tx_done loop pushes the skb before the actual ACK for that index happens. Fixed both issues.
As you mentioned, I have introduced a lock to protect this critical section. I'll send the updates in the next v2 patch.

Thanks,
Ramesh