Ira W. Snyder wrote: > This patch series adds support for the Janz CMOD-IO carrier board, as well > as the Janz VMOD-ICAN3 Intelligent CAN controller. The CMOD-IO carrier > board is a PCI to MODULbus bridge, into which plug MODULbus daughterboards. > I only have access to two types of daughtercards, the VMOD-ICAN3 mentioned > above, and the VMOD-TTL GPIO controller. > > All of my boards have two VMOD-ICAN3 modules and one VMOD-TTL module. This > posting only contains drivers for the CMOD-IO carrier board and VMOD-ICAN3 > CAN interfaces. A driver for the VMOD-TTL GPIO module is under development, > and will be posted shortly. This module is even worse to program nicely > than the ICAN3 module. > > Since the RFCv2 posting, the CAN driver has been much more thoroughly > tested. CAN bus-off works correctly, as does the generation of error > frames. The bus-off and error frame code has been adapted from the SJA1000 > driver, as the ICAN3 firmware reports most of the status registers used by > the SJA1000 code.
Sounds good and from my point of view the driver is more or less ready for mainline inclusion. If that is your primary goal and you feel it is mature and stable enough, please send a proper patch series as described here: http://svn.berlios.de/svnroot/repos/socketcan/trunk/README.submitting-patches. As an alternative, I could apply it to the SVN trunk for the time being. There, the requirements for acceptance are not that high. I briefly browsed the patches. Here some quick comments: - I do still not find __devinit, __devexit, and friends in your drivers as described here: http://lxr.linux.no/#linux+v2.6.32/Documentation/PCI/pci.txt#L177 They are also missing in janz-ican3.c. - You may need to declare some structures "__attribute__((packed))", - 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. - Some time ago we agreed to use "_" for the Socket-CAN file names: s/janz-ican3/janz_ican3/ - You still use many hard-code numbers in the code. Please define values for most of them to make the code more readable. - There are still to much dev_dbg(). They should especially not be used in the xmit and recv path. - I see still a lot of duplicated code, especially for desc handling. Maybe some helper functions or combined i/o functions for send/recv could make the code more compact. - Checkpatch reports "lines too long". - s+<linux/janz.h>+<linux/mfd/janz.h>+ ? - Check MODULE_LICENSE(). It does not match with your copyright notes. - 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? Wolfgang. _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
