Hi Matthias,

Matthias Fuchs wrote:
> Hi,
> 
> here comes version 2 of the esd_usb2 driver. I tried to consider
> nearly all of Wolfgang's and Marc's comments. Thank's for those.
> 
> But I ran into some issues. Perhaps some of you can help:
> Q1: Under high TX load the driver runs out of free tx_contexts.
> (see "couldn't find free context"). I expected the socket can core
> to take care of this so that the lower driver cannot be overrun
> by tx jobs. Either this expectation is wrong or there's a bug in
> the driver :-)

As already mentioned. The netdev tx queue start/stop should be handled
in the tx_done function.

> Q2: Marc commented on start_xmit returning NETDEV_TX_OK even
> on sometinhg like an error. This seems to be common practice.
> Othr drivers do the same. So can we keep this or did I miss
> something?

That's almost perfect as pointed out by Oliver here:
https://lists.berlios.de/pipermail/socketcan-core/2010-January/003777.html
Just use kfree_skb(skb) instead of dev_kfree_skb(skb).

> Q3: I still get a segfault during module unloading (most times).
> This is caused by usb_get_urb() that called from 
> usb_kill_anchored_urbs(&priv->tx_submitted). When nobody has an
> idea, I will wait until the other stuff is more or less
> fine form the socketcan side and then post it to the usb
> people for review.

Can't help here.

> Q4: I don't see that adding macros for all constants that are used for the
> bitrate/btr calculation will make the code look better. The SJA1000 code
> does the same and its definetly a place where others should keep hands off :-)

I personally disagree. It would be nice for the SJA1000 to have here
macro definitions as well. But it's currently a minor issue, I agree.

Wolfgang.
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core

Reply via email to