Hi Marc, Sorry for the late response. > On 03/02/2016 11:08 AM, Ramesh Shanmugasundaram wrote: > >>>> 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
