On Wed, Feb 10, 2010 at 07:45:30PM +0100, Wolfgang Grandegger wrote: > 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. >
I am the only author of the driver posted in the first email of the thread. I have written it using only the datasheets provided by Janz on their website. For many years at my organization, we have been using Linux char drivers provided by Janz. We would like to move to a driver which is present in mainline Linux, to make switching kernels easier. > > 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. > I'll get the rest of the driver working before I worry about this. > >>> - 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. > Yeah, it sucks. Nothing I can do about it, though. That's how the hardware is... > > 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. > It is easy enough to implement. I'll do that for the next posting of the driver. > >> - 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? > Nope. If there is mention of it in the manual, I cannot find it. :( I *do* have a notification mechanism if I don't support EFF CAN frames, but I'd really like to have support for that. I could read the entire ring off the device after each xmit() and see if it is full. But then I have no way of knowing when to start the queue again, unless I set a kernel timer and keep reading the ring. That is a terribly slow (and ugly) approach, though. > >> - 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. > Ok, I'll try and figure out what these do. The comments in the code /really/ make it seem like these are for local loopback. The descriptions even talk about local loopback! A better description might help authors such as myself. >From drivers/net/can/dev.c: /* * Get the skb from the stack and loop it back locally * * The function is typically called when the TX done interrupt * is handled in the device driver. The driver must protect * access to priv->echo_skb, if necessary. */ void can_get_echo_skb(struct net_device *dev, unsigned int idx) /* * Put the skb on the stack to be looped backed locally lateron * * The function is typically called in the start_xmit function * of the device driver. The driver must protect access to * priv->echo_skb, if necessary. */ void can_put_echo_skb(struct sk_buff *skb, struct net_device *dev, unsigned int idx) > >> 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. > Thanks, Ira _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
