On 20.2.2017 12:27, Anssi Hannula wrote: > On 20.2.2017 11:42, Marc Kleine-Budde wrote: >> On 02/13/2017 04:39 PM, Anssi Hannula wrote: >>> The xilinx_can driver assumes that the TXOK interrupt only clears after >>> it has been acknowledged as many times as there have been successfully >>> sent frames. >>> >>> However, the documentation does not mention such behavior, instead >>> saying just that the interrupt is cleared when the clear bit is set. >>> >>> Similarly, testing seems to also suggest that it is immediately cleared >>> regardless of the amount of frames having been sent. Performing some >>> heavy TX load and then going back to idle has the tx_head drifting >>> further away from tx_tail over time, steadily reducing the amount of >>> frames the driver keeps in the TX FIFO (but not to zero, as the TXOK >>> interrupt always frees up space for 1 frame from the driver's >>> perspective, so frames continue to be sent) and delaying the local echo >>> frames. >>> >>> The TX FIFO tracking is also otherwise buggy as it does not account for >>> TX FIFO being cleared after software resets, causing >>> BUG!, TX FIFO full when queue awake! >>> messages to be output. >>> >>> Since there does not seem to be any way to accurately track the state of >>> the TX FIFO, drop the code trying to do that and perform >>> netif_stop_queue() based on whether the HW reports the FIFO being full. >>> Driver-side local echo support is also removed as it is not possible to >>> do accurately (relying instead on the CAN core fallback support). >>> >>> Tested with the integrated CAN on Zynq-7000 SoC. >>> >>> v2: Add FIFO space check before TX queue wake with locking to >>> synchronize with queue stop. This avoids waking the queue when xmit() >>> had just filled it. >> Removing the echo on TX-complete should be avoided if possible. Is there >> a way to get a indication that an individual package has been send? > Looking at the reference manual [1, chapter 18 and appendix B.5] I can't > see any. The TX FIFO interrupts and status registers available are TXOK > (1 or more messages sent), FIFO empty, FIFO full, FIFO under watermark > (watermark not programmable while on-bus). There is no FIFO free space > counter that I can see. > > One way would be to only put a single frame into the FIFO at a time, of > course, but at least to me that seems worse than losing echo-on-TX-complete. A correction to my last statement above, we could probably put up to 3 frames in TX FIFO at a time and still keep track of them using the TX FIFO watermark, allowing echo-on-TX-complete to still be supported, with something like this: if (tx fifo empty) all frames sent; else if (tx fifo under watermark 2) all except 1 frame sent; else if (txok) 1 frame sent; (all except 2) Though 3 is still much less than 64, the current TX FIFO max size. I'd myself prefer keeping full FIFO over keeping echo-on-TX-complete, but not sure what is the general opinion. > [1] > https://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf > > -- Anssi Hannula / Bitwise Oy +358503803997