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
��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
