Thanks Marc for your feedback! On 07/19/2019 03:27 PM, Marc Kleine-Budde wrote: > Can you convert the driver from a ldisc to a serdev driver? I would have preferred that, but unfortunately this is not an option, because: a) There is no way to autoprobe this driver. The serial ports it gets attached to are as generic as they come. Think "bog-standard PL2303 USB-serial converter that looks like any other", and next week it may be a CH340 instead. b) The user may well want to initialise the device beforehand, e.g. to increase the baud rate. For the above reasons, attachment needs to be initiated from userspace, using something like ldattach. I haven't found an equivalent for serdev, but please tell me if I've missed something. As I said, I'd prefer a serdev over an ldisc if it's feasible. > There is also the problem of using netif_rx_ni(skb) to push the frames > into networking stack. With this function the CAN frames might not end > up in the user space in the correct order. You have to use > netif_receive_skb(skb) to avoid this problem. That's a very helpful hint, I didn't know that, and it must be a common mistake: There is a *ton* of CAN drivers using either netif_rx_ni or netif_rx(), and I built on their style. Does that mean they are all wrong in this way, and this problem was only caught recently? > However this is a bit more complicated, as you can only use this > function from the NAPI context. You can setup a rx_offload, via > can_rx_offload_add_fifo() and then use can_rx_offload_queue_tail() to > push the CAN frames to the networking stack. See the flexcan driver as > an example. Use this [1] version as a reference how to use > can_rx_offload_queue_tail(). It's used here for error frames, but you > can use it for normal frames as well. Thanks for this help, I'll look into implementing this. Max
