Ira W. Snyder wrote: > On Wed, Feb 10, 2010 at 01:09:53PM +0100, Wolfgang Grandegger wrote: >> Ira W. Snyder wrote: >>> Hello all, >>> >>> I posted last week asking about a driver for boards running on a PLX >>> chip. It turns out that these are passive boards. I have been looking >>> for a driver for the Janz CMOD-IO board CAN interfaces. >>> >>> I finally found the datasheets and took the time to write a driver. This >>> board is an intelligent CAN interface: it has onboard microprocessors to >>> help process CAN traffic. >>> >>> This is a very rough first try at a CAN driver. I'm sure it still has >>> problems, and I would appreciate if you can take a look and help me add >>> what is needed. I'm am not extremely knowledgeable about the CAN bus. >> You are welcome. I think the esd_pci331 drive should answer most of your >> questions. >> >>> The things that are known to be wrong: >>> >>> - bus-on / bus-off handling >>> >>> I did this straight in the network device open()/stop() methods. I don't >>> handle the condition where we get too many bus errors and the bus goes >>> into bus-off state. What should I be doing here? >> Search for "can_bus_off" in esd_pci331.c. The controller (hardware) goes >> bus-off if to much errors occurred and it's necessary to handle that >> event in the driver and report it to the apps. Yon can only recover from >> a bus-off initiating a so called bus-off recovery, which is done via >> esd331_set_mode(CAN_MODE_START). >> >>> - state changes, bit timing, etc. >> Search for "alloc_can_err_skb" and "CAN_ERR" in esd_pci331.c. We report >> CAN error state changes and errors via so called CAN error messages, >> which the apps can receive on request. >> >>> I'm not at all sure how this is supposed to work. Perhaps someone that >>> knows CAN bus better that I do can help. >>> >>> - CAN bus termination >>> >>> This board supports optionally terminating the CAN bus. In order to test >>> this driver, I connected both CAN modules on a single board together. To >>> get this to work, I needed to turn on termination on both modules. >>> >>> Is this wrong? Is there a way to enable/disable termination via the "ip" >>> tool? >> No, that's specific to this driver and could be implemented via SysFS, >> as Kurt suggested. >> > > Ok. All of the existing code I have that uses Janz's char driver enables > termination. I think just enabling termination (as I did in my driver)
The copyright made me think that you are the author of this code. > will be fine. If I find a need for switching off termination later > (unlikely) then I'll add a sysfs attribute. That is a bad idea. You must use termination if the controller is at one end of the bus, which might not always be the case. But that's not that important in the first place. >>> - module probing >>> >>> This board is really a MODULbus carrier board, into which plugs 4 >>> daughterboards. These can be CAN modules, or others. On my board, I have >>> 2x CAN modules and 1x TTL GPIO module. I need support for both of these >>> for my application. For the time being, I *did not* add support for the >>> TTL module. That will come once the CAN part is finished. >> The modules are connected to the PCI bus, I assume. The TTL module >> should be handled by a dedicated driver. >> > > The modules are NOT directly connected to the PCI bus, they connected to > PCI through a multi-slot carrier board. They are "MODULbus" modules, > which are very much NOT PCI modules. The carrier board IS a normal PCI > board. It has a PLX 9030/9050 PCI bridge chip onboard. This chip bridges > between PCI <-> MODULbus. > > The modules are accessed by reading specific memory addresses in PCI > BAR3. The formula is bar3_address + (module_number * 0x200). > > It looks like this: > http://www.janz.de/as/en/cmod-io.html I already realized that. The clean solution would be to have a driver supporting the MODULbus and the CAN or other drivers would then connect to that bus/driver. But that's another effort and maybe overkill for your purpose. >>> Also, there is no way I am aware of to determine what type of board is >>> plugged into which MODULbus slot on the carrier board. My CAN modules >>> support identification, but the TTL module does not. >>> >>> I hard-coded the module layout into the driver itself. This is >>> sufficient for my purposes, but probably is not sufficient for mainline >>> Linux :'(. Any ideas or suggestions? >> This information could be provide via module parameter, for example. >> >>> Any review will be much appreciated! >> Puh, that's a lot of untested code. Some general comments/questions: >> >> - You use NAPI and a work-queue. Why do you need both? >> > > This device's programming interface has three different styles of > communication with the driver. > > 1) "old-style" host interface (control + SFF CAN messages) > 2) "new-style" host interface (control + SFF CAN messages) > 3) "fast" host interface (SFF + EFF CAN messages only) > > In order for normal operation, you must be able to send/recv both > control messages and SFF + EFF CAN frames. > > Only the "old-style" interface is enabled when the chip comes out of > reset. You must use it to enable the "new-style" interface. Then the > "new-style" interface must be used to enable the "fast" interface. > Follow janz_startup_module() for the details. Puh. > I used the workqueue to process "old-style" and "new-style" control > messages. They indicate things like CAN messages lost and other CAN > events. These are typical candidates to create error messages. > I used NAPI to process CAN frames only. OK. > I could remove NAPI support and roll the functionality into the > workqueue, if you think that would be better. NAPI support is a good thing in general but having it in the work queue seems more straight forward and you could get rid of a lot of synchronization, I assume. But that's just a wild guess for the moment. >> - I do not see how you control the flow of TX messages? To be more >> precise, I do not find netif_stop_queue() in your xmit path. >> > > I don't. :( > > Unfortunately, AFAICT the "fast" interface doesn't support any sort of > notification to tell the driver when it consumes a buffer. The > "new-style" interface does have support for notification, but it doesn't > support send/recv of EFF CAN frames. > > Do you have ideas of how to do this without any indication of "fullness" > from the hardware? You could at least restrict the number of messages queued. Does the firmware provide a TX done notification? >> - You can use the standard bit-timing calculation. See how it's done in >> sja1000/sja1000.c. Search for sja1000_bittiming_const and >> sja1000_set_bittiming. >> >> - You do not handle echo skb's. Search for can_get_echo_skb() and >> can_put_echo_skb() for local loopback. >> > > Shouldn't this be ommitted if the CAN controller supports echoing the > frame back as a normal recv'd frame when it is sent? My controller does > this. If the hardware does the job of looping back the local packet then it should not be done in software, of course. > I thought the can_(get|put)_echo_skb() functionality was the Linux CAN > layer software fallback. Am I wrong? No. >> As I see it, this code does not run or even compile yet... too early for >> a full review. >> > > It both compiles (against mainline 2.6.33-rc7) and runs! I wouldn't have > sent it if it did not! The can-utils cansend and candump programs were > used to test the device. I hooked a CAN cable between two modules to > test. Some lines of code made me think so. But I just tried. It compiles fine. Sorry for the noise. Will comment on the code later. > Running cansend to send a CAN frame out can0, candump sees the frame > appear both on can0 (echo frame) and can1 (recv'd across the wire). Both > SFF and EFF frames were tested. Isn't that the correct behavior? Sound good. Wolfgang. _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
