On 02/21/2017 03:57 PM, Anssi Hannula wrote: > 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. Sounds good, can you work on a patch? > I'd myself prefer keeping full FIFO over keeping echo-on-TX-complete, > but not sure what is the general opinion. Big FIFOs have the drawback of bigger latencies, as you cannot reorder high priority packages inside the kernel. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Attachment:
signature.asc
Description: OpenPGP digital signature