Ira W. Snyder wrote:
> On Mon, Feb 22, 2010 at 08:42:56PM +0100, Wolfgang Grandegger wrote:
>>
>> - Don't include sja1000/sja1000.h. It's only for drivers in sja1000.
>> I know that some other drivers use SJA1000 definitions as well, but
>> that requires a general solution.
>>
>
> Why not? I need some of the definitions for the SJA1000 error registers.
> Is there any reason why it can't be include/linux/can/sja1000.h instead?
>
> It seems stupid to duplicate the register definitions in each new driver
> that comes along.
Register definitions are always hardware specific.
The fact that the Janz card has some registers in SJA1000 layout does not
justify to push sja1000.h to a public place like include/linux/can/* IMO.
The ESD USB and the EMS USB drivers also have some registers in SJA1000
layout. I would prefer following their approach.
>
>> - About xmit flow control. What happens if you send messages quickly at
>> 125 KB/s. You could use "cangen -g 0 can0" for that test. How many
>> messages get dropped?
>>
>
> I let the cangen command run for a while:
> $ ifconfig -a
> can0 Link encap:UNSPEC HWaddr
> 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00
> UP RUNNING NOARP MTU:16 Metric:1
> RX packets:473455 errors:0 dropped:0 overruns:0 frame:0
> TX packets:473455 errors:0 dropped:1831983 overruns:0 carrier:0
Oops!
This looks weird in janz_xmit() !
+ /* check that we can actually transmit */
+ if (!(desc.control & DESC_VALID)) {
+ dev_err(mod->dev, "%s: no buffers\n", __func__);
+ stats->tx_dropped++;
+ kfree_skb(skb);
+ goto out_unlock;
+ }
You should enable/wake the tx_queue only when your hardware is potentially
able to process new CAN frames. You are getting lot's of trashed frames with
your implementation.
Regards,
Oliver
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
