This patch adds the virtual CAN bus (vcan) network driver. The vcan device is just a loopback device for CAN frames, no real CAN hardware is involved. Signed-off-by: Oliver Hartkopp <oliver.hartkopp@volkswagen.de> Signed-off-by: Urs Thuermann <urs.thuermann@volkswagen.de> --- drivers/net/Makefile | 1 drivers/net/can/Kconfig | 25 +++++ drivers/net/can/Makefile | 5 + drivers/net/can/vcan.c | 207 +++++++++++++++++++++++++++++++++++++++++++++++ net/can/Kconfig | 3 5 files changed, 241 insertions(+) Index: net-2.6.24/drivers/net/Makefile =================================================================== --- net-2.6.24.orig/drivers/net/Makefile 2007-09-20 23:46:01.000000000 +0200 +++ net-2.6.24/drivers/net/Makefile 2007-10-02 12:03:30.000000000 +0200 @@ -12,6 +12,7 @@ obj-$(CONFIG_CHELSIO_T1) += chelsio/ obj-$(CONFIG_CHELSIO_T3) += cxgb3/ obj-$(CONFIG_EHEA) += ehea/ +obj-$(CONFIG_CAN) += can/ obj-$(CONFIG_BONDING) += bonding/ obj-$(CONFIG_ATL1) += atl1/ obj-$(CONFIG_GIANFAR) += gianfar_driver.o Index: net-2.6.24/drivers/net/can/Kconfig =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ net-2.6.24/drivers/net/can/Kconfig 2007-10-02 12:03:30.000000000 +0200 @@ -0,0 +1,25 @@ +menu "CAN Device Drivers" + depends on CAN + +config CAN_VCAN + tristate "Virtual Local CAN Interface (vcan)" + depends on CAN + default N + ---help--- + Similar to the network loopback devices, vcan offers a + virtual local CAN interface. + + This driver can also be built as a module. If so, the module + will be called vcan. + +config CAN_DEBUG_DEVICES + bool "CAN devices debugging messages" + depends on CAN + default N + ---help--- + Say Y here if you want the CAN device drivers to produce a bunch of + debug messages to the system log. Select this if you are having + a problem with CAN support and want to see more of what is going + on. + +endmenu Index: ...
Can debug be a boolean? Like its counterpart on DCCP: net/dccp/proto.c: module_param(dccp_debug, bool, 0444); Where we also use a namespace prefix, for those of us who use ctags or Is this comment still valid? If so can this move happen now? If not I Thinking out loud: I guess these days we can try to reduce the clutter on the source code for things like "hey, I entered function foo" using simple systemtap scripts, that could even be shipped with the kernel -
Bringing all the current available CAN network device drivers into Kernel style qualitiy is a TODO for the time after the PF_CAN core is mainlined. When more than this single vcan CAN netdev driver is part of the Kernel it makes sense to put several things (like the common configuration interface and commonly used library funtions for CAN drivers) into linux/can/dev.h. And at that time this currently local DEBUG definition should go there togehther with the other stuff. Please think of all the comments, if we created a single dev.h file with a single DEBUG definition used by a single vcan.c file ;-) Oliver -
Don't get to defensive. You know a lot more than me about the code you worked for that many years. Its just that it looked suspicious for a casual reviewer. :-) - Arnaldo -
'debug' should remain an integer to be able to specifiy debug-levels or Even if i don't have any general objections to rename this 'debug' to 'vcan_debug', it looks like an 'overnamed' module parameter for me. Is Yes. This is definitely a boolean candidate. We'll change that. Thanks, Oliver -
[acme@mica linux-2.6.23-rc9-rt1]$ find . -name "*.c" | xargs grep 'module_param(.\+debug,' | wc -l 112 [acme@mica linux-2.6.23-rc9-rt1]$ find . -name "*.c" | xargs grep 'module_param(debug,' | wc -l 233 [acme@mica linux-2.6.23-rc9-rt1]$ I think that helping ctags to find the definition for the debug variable to see, for instance, if it is a bitmask or a boolean without having to chose from tons of 'debug' variables is a good thing. - Arnaldo -
From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net> I completely agree. -
OK. No problem if it's helpful. We'll change debug to vcan_debug. Thanks. Oliver -
On Tue, 02 Oct 2007 23:02:53 +0200 Please consider using netif_msg_xxx() and module parameter to set default message level, like other real network drivers already do. -- Stephen Hemminger <shemminger@linux-foundation.org> -
From: Stephen Hemminger <shemminger@linux-foundation.org> I keep seeing this recommendation, but the two supposedly most mature and actively used drivers in the tree, tg3 and e1000 and e1000e, all do not use this scheme. In fact there are tons of drivers that even hook up the ethtool msg_level setting function and never even use the value. If people aren't using netif_msg_xxx() and the ethtool msg_level facilities properly, it's because there is a severe dearth of good example drivers to learn about it from. -
The currently available CAN netdevice drivers do not have a common debug concept neither any runtime control mechanism for this debugging. So netif_msg_xxx() is definitely worth to look at instead of creating any new stuff in this direction, before posting any 'real' CAN network driver here. Thanks very much for that hint! Oliver -
debug used to a bit mask, like it still is in core.h. You can see
this in the test
debug & 1 ? ... : ...
I think ctags should be able to handle multiple identical static
symbols. Isn't it? I find it somewhat clumsy to write
modprobe vcan vcan_debug=1
I think it would be nice to change the module_param() macro so that
you can name the module argument and the corresponding variable
independently, like
module_param(can_debug, "debug", bool, 0444);
OK, forget that last paragraph. I've looked at the definition of
module_param() and have seen that we have module_param_named(). I
I've never heard of systemtap before. I've ust looked at its overview
web page which sounds promising. I think I'll check it out when time
permits...
urs
-
